From: Chandler Carruth <chandlerc@gmail.com>
Date: Wed, 18 Oct 2017 22:42:36 +0000 (+0000)
Subject: [PM] Refactor the bounds checking pass to remove a method only called in
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7121c763ecfeb89db8428633055abc70066bd048;p=llvm

[PM] Refactor the bounds checking pass to remove a method only called in
one place.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@316135 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Transforms/Instrumentation/BoundsChecking.cpp b/lib/Transforms/Instrumentation/BoundsChecking.cpp
index a193efe902c..4a7875b6d48 100644
--- a/lib/Transforms/Instrumentation/BoundsChecking.cpp
+++ b/lib/Transforms/Instrumentation/BoundsChecking.cpp
@@ -60,7 +60,6 @@ namespace {
     BasicBlock *TrapBB;
 
     BasicBlock *getTrapBB();
-    void emitBranchToTrap(Value *Cmp = nullptr);
     bool instrument(Value *Ptr, Value *Val, const DataLayout &DL);
  };
 }
@@ -92,32 +91,6 @@ BasicBlock *BoundsChecking::getTrapBB() {
 }
 
 
-/// emitBranchToTrap - emit a branch instruction to a trap block.
-/// If Cmp is non-null, perform a jump only if its value evaluates to true.
-void BoundsChecking::emitBranchToTrap(Value *Cmp) {
-  // check if the comparison is always false
-  ConstantInt *C = dyn_cast_or_null<ConstantInt>(Cmp);
-  if (C) {
-    ++ChecksSkipped;
-    if (!C->getZExtValue())
-      return;
-    else
-      Cmp = nullptr; // unconditional branch
-  }
-  ++ChecksAdded;
-
-  BasicBlock::iterator Inst = Builder->GetInsertPoint();
-  BasicBlock *OldBB = Inst->getParent();
-  BasicBlock *Cont = OldBB->splitBasicBlock(Inst);
-  OldBB->getTerminator()->eraseFromParent();
-
-  if (Cmp)
-    BranchInst::Create(getTrapBB(), Cont, Cmp, OldBB);
-  else
-    BranchInst::Create(getTrapBB(), OldBB);
-}
-
-
 /// instrument - adds run-time bounds checks to memory accessing instructions.
 /// Ptr is the pointer that will be read/written, and InstVal is either the
 /// result from the load or the value being stored. It is used to determine the
@@ -158,8 +131,32 @@ bool BoundsChecking::instrument(Value *Ptr, Value *InstVal,
     Value *Cmp1 = Builder->CreateICmpSLT(Offset, ConstantInt::get(IntTy, 0));
     Or = Builder->CreateOr(Cmp1, Or);
   }
-  emitBranchToTrap(Or);
 
+  // check if the comparison is always false
+  ConstantInt *C = dyn_cast_or_null<ConstantInt>(Or);
+  if (C) {
+    ++ChecksSkipped;
+    // If non-zero, nothing to do.
+    if (!C->getZExtValue())
+      return true;
+  }
+  ++ChecksAdded;
+
+  BasicBlock::iterator SplitI = Builder->GetInsertPoint();
+  BasicBlock *OldBB = SplitI->getParent();
+  BasicBlock *Cont = OldBB->splitBasicBlock(SplitI);
+  OldBB->getTerminator()->eraseFromParent();
+
+  if (C) {
+    // If we have a constant zero, unconditionally branch.
+    // FIXME: We should really handle this differently to bypass the splitting
+    // the block.
+    BranchInst::Create(getTrapBB(), OldBB);
+    return true;
+  }
+
+  // Create the conditional branch.
+  BranchInst::Create(getTrapBB(), Cont, Or, OldBB);
   return true;
 }