[JumpThreading] Replace uses of Condition safely

[anna]

Summary

This patch builds over https://reviews.llvm.org/rL303349 and replaces the use of the condition only if it is safe to do so.

We should not blindly RAUW the condition if experimental.guard or assume is a use of that
condition. This is because LVI may have used the guard/assume to identify the
value of the condition, and RUAWing will fold the guard/assume and uses before the guards/assumes.

[anna]

lib/Transforms/Scalar/JumpThreading.cpp

258 ↗

(On Diff #99193)

This should be "We can safely RAUW Cond only if ...".
I have separated this out into a function because there are 2 places currently where we try to RAUW the condition. Tests added trigger both these source code cases: ProcessBlock and ProcessThreadableEdges.

[sanjoy]

lib/Transforms/Scalar/JumpThreading.cpp

266 ↗

(On Diff #99193)

I don't think this is specific to guards. For instance, if you have:

bb:
  cond = icmp ...;
  print(cond);
  exit();
  assume(cond);
  br cond;

You can't simplify the cond in print(cond) to print(true). I think there are similar issues with isObjectDereferencedInBlock.

I'd instead phrase this problem as:

We know that at the end of a basic block, a certain condition, cond, is true. The question from what point onwards did that condition become true. To do that, I suggest walking backwards from the branch, and stopping when you find an instruction for which isGuaranteedToTransferExecutionToSuccessor returns false. All the instructions you enumerate this way can be replaceUsesOfWith(cond, true) 'ed. This approach will miss some cases though, like in:

bb:
  cond = icmp ...;
  print(cond);
  cond' = or cond, something_else
  exit();
  use(cond')
  assume(cond);
  br cond;

we won't simplify cond'. If needed, we can come up with a more aggressive substitution scheme to get those cases.

[reames]

Anna, I believe this is a near duplicate of a patch I posted a while ago and never got around to getting in: https://reviews.llvm.org/D28276. I'm happy to have you take this to completion, but you might find the code and previous review comments there helpful.

In retrospect, I think I made a mistake in how I approached this. This is an active bug. We should remove the replaceAllUsesWith and update the tests as needed. Only once that's done should we come back and see if we can selectively update in some cases.

[anna]

In D33257#756904, @reames wrote:

Anna, I believe this is a near duplicate of a patch I posted a while ago and never got around to getting in: https://reviews.llvm.org/D28276. I'm happy to have you take this to completion, but you might find the code and previous review comments there helpful.

In retrospect, I think I made a mistake in how I approached this. This is an active bug. We should remove the replaceAllUsesWith and update the tests as needed. Only once that's done should we come back and see if we can selectively update in some cases.

Philip, your patch is almost around the same issue! I was under the impression this circular logic affects guards only, but as Sanjoy pointed out it can be assumes as well (because LVI calculates values based on assumes and guards).

So, I'll remove the RAUW in the failing cases, and update the test cases asap. Next step would be following Sanjoy's idea/Philip's patch to reason about aggressively updating in cases where it is safe to do so.

[mkazantsev]

lib/Transforms/Scalar/JumpThreading.cpp

270 ↗

(On Diff #99193)

Should we check all dominating blocks as well?

[anna]

lib/Transforms/Scalar/JumpThreading.cpp

270 ↗

(On Diff #99193)

No, we don't need to check blocks dominating BB because the bug occurs when guard/assume *use* the Cond. Since defs dominate uses, we are fine.
Also, we don't need to check for uses in blocks dominated by BB because starting from the point of BB's terminator (i.e. the context instruction passed into LVI) and iterating forward, we can safely replace the uses of Cond with Val (returned by LVI).

The issue is when we try to replaces uses within the block BB where Cond is defined.

[anna]

lib/Transforms/Scalar/JumpThreading.cpp

266 ↗

(On Diff #99193)

Sanjoy's phrasing is a bit different from this phrasing: do not fold guards and assumes and uses before the guards and assumes.

Specifically these 2 examples showcase the differences:

1. Using isGuaranteedToTransferExecutionToSuccessor, we will be folding assumes. Folding to true is okay, based on lang ref semantics of assume. Also, I think there cannot be a case of folding to assume(false), based on how LVI calculates values.

bb:
  cond = icmp ...;
  print(cond); <-- can fold this, and we do so.
  assume(cond); <-- can fold this, and we do so.
  br cond;

If there was an exit() between print and assume, we cannot fold the print.

2. In this test case below, we unfortunately do not fold either uses because exit is a throwing call, and isGuaranteedToTransferExecutionToSuccessor won't allow to fold.

bb:
  cond = icmp ...
  assume(cond) <-- can fold this, but not able to.
  print(cond) <-- can fold this, but not able to.
  exit() <-- throws
  br cond

I think either phrasing has it's pros and cons, but both are conservatively correct. Thoughts?

[reames]

This needs rebased on top of https://reviews.llvm.org/D33279.

[mkazantsev]

lib/Transforms/Scalar/JumpThreading.cpp

270 ↗

(On Diff #99193)

Ok, thanks for explanation!

[anna]

Replace uses of condition safely. The updated algorithm follows suggestion from Sanjoy:
starts from the end of basic block where the condition is defined and stops
when we reach a statement that may trap.
Added testcases in assumes and guards to show which cases we currently capture and which we do not.
Rebased over https://reviews.llvm.org/D33279.

[sanjoy]

Mostly minor stuff.

lib/Transforms/Scalar/JumpThreading.cpp

263 ↗

(On Diff #99448)

I'd call this replaceFoldableUses. IMHO the key transform here is that you're replacing uses, the fact that you can also sometimes remove Cond is just a "fringe benefit".

277 ↗

(On Diff #99448)

Perhaps this can be Instruction &I : reverse(*BB)?

283 ↗

(On Diff #99448)

I'd break out of the loop in both cases, and erase Cond only if it does not have uses. That way we'll remove the condition in cases like:

cond = icmp;
exit(0);
use(cond);
assume(cond);

[anna]

Addressed review comments.

[sanjoy]

lgtm with nits

lib/Transforms/Scalar/JumpThreading.cpp

264 ↗

(On Diff #99476)

PatternMatch is unused.

268 ↗

(On Diff #99476)

The non-local blocks need not be strictly dominated by BB, but the uses we replace will be. The non-local blocks will not be strictly dominated in case of PHI nodes uses:

bb1:
  br label %x

bb0:
  assume(%cond)
  br i1 %cond, label %x, label %y

x:
  %v = phi i1 [ %cond, %bb0 ], [ %other_cond, %bb1 ]

I.e. the code is correct, but the comment can be more accurate.

271 ↗

(On Diff #99476)

This is a very subjective stylistic thing, but I think this much explanation is unnecessary. I'd just say something brief like "We only replace uses in instructions that are guaranteed to reach the end of the block, where we know Cond is ToVal." right before the check on isGuaranteedToTransferExecutionToSuccessor -- we have test cases to show why this is needed.

1376 ↗

(On Diff #99476)

Braces unnecessary here.

[anna]

lib/Transforms/Scalar/JumpThreading.cpp

268 ↗

(On Diff #99476)

good point!

llvm/trunk/include/llvm/Transforms/Utils/Local.h

 void combineMetadata(Instruction *K, const Instruction *J, ArrayRef<unsigned> KnownIDs);
 
 /// Combine the metadata of two instructions so that K can replace J. This
 /// specifically handles the case of CSE-like transformations.
 ///
 /// Unknown metadata is removed.
 void combineMetadataForCSE(Instruction *K, const Instruction *J);
 
+// Replace each use of 'From' with 'To', if that use does not belong to basic
+// block where 'From' is defined. Returns the number of replacements made.
+unsigned replaceNonLocalUsesWith(Instruction *From, Value *To);
+
 /// Replace each use of 'From' with 'To' if that use is dominated by
 /// the given edge.  Returns the number of replacements made.
 unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT,
                                   const BasicBlockEdge &Edge);
 /// Replace each use of 'From' with 'To' if that use is dominated by
 /// the end of the given BasicBlock. Returns the number of replacements made.
 unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT,
                                   const BasicBlock *BB);

llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp

do {
     }
     EverChanged |= Changed;
   } while (Changed);
 
   LoopHeaders.clear();
   return EverChanged;
 }
 
+// Replace uses of Cond with ToVal when safe to do so. If all uses are
+// replaced, we can remove Cond. We cannot blindly replace all uses of Cond
+// because we may incorrectly replace uses when guards/assumes are uses of
+// of `Cond` and we used the guards/assume to reason about the `Cond` value
+// at the end of block. RAUW unconditionally replaces all uses
+// including the guards/assumes themselves and the uses before the
+// guard/assume.
+static void ReplaceFoldableUses(Instruction *Cond, Value *ToVal) {
+  assert(Cond->getType() == ToVal->getType());
+  auto *BB = Cond->getParent();
+  // We can unconditionally replace all uses in non-local blocks (i.e. uses
+  // strictly dominated by BB), since LVI information is true from the
+  // terminator of BB.
+  replaceNonLocalUsesWith(Cond, ToVal);
+  for (Instruction &I : reverse(*BB)) {
+    // Reached the Cond whose uses we are trying to replace, so there are no
+    // more uses.
+    if (&I == Cond)
+      break;
+    // We only replace uses in instructions that are guaranteed to reach the end
+    // of BB, where we know Cond is ToVal.
+    if (!isGuaranteedToTransferExecutionToSuccessor(&I))
+      break;
+    I.replaceUsesOfWith(Cond, ToVal);
+  }
+  if (Cond->use_empty() && !Cond->mayHaveSideEffects())
+    Cond->eraseFromParent();
+}
+
 /// Return the cost of duplicating a piece of this block from first non-phi
 /// and before StopAt instruction to thread across it. Stop scanning the block
 /// when exceeding the threshold. If duplication is impossible, returns ~0U.
 static unsigned getJumpThreadDuplicationCost(BasicBlock *BB,
                                              Instruction *StopAt,
                                              unsigned Threshold) {
   assert(StopAt->getParent() == BB && "Not an instruction from proper BB?");
   /// Ignore PHI nodes, these will be flattened when duplication happens.
if (CondBr && CondConst) {
       if (Ret != LazyValueInfo::Unknown) {
         unsigned ToRemove = Ret == LazyValueInfo::True ? 1 : 0;
         unsigned ToKeep = Ret == LazyValueInfo::True ? 0 : 1;
         CondBr->getSuccessor(ToRemove)->removePredecessor(BB, true);
         BranchInst::Create(CondBr->getSuccessor(ToKeep), CondBr);
         CondBr->eraseFromParent();
         if (CondCmp->use_empty())
           CondCmp->eraseFromParent();
-        // TODO: We can safely replace *some* uses of the CondInst if it has
+        // We can safely replace *some* uses of the CondInst if it has
         // exactly one value as returned by LVI. RAUW is incorrect in the
         // presence of guards and assumes, that have the `Cond` as the use. This
         // is because we use the guards/assume to reason about the `Cond` value
         // at the end of block, but RAUW unconditionally replaces all uses
         // including the guards/assumes themselves and the uses before the
         // guard/assume.
+        else if (CondCmp->getParent() == BB) {
+          auto *CI = Ret == LazyValueInfo::True ?
+            ConstantInt::getTrue(CondCmp->getType()) :
+            ConstantInt::getFalse(CondCmp->getType());
+          ReplaceFoldableUses(CondCmp, CI);
+        }
         return true;
       }
 
       // We did not manage to simplify this branch, try to see whether
       // CondCmp depends on a known phi-select pattern.
       if (TryToUnfoldSelect(CondCmp, BB))
         return true;
     }
if (PredWithKnownDest ==
       BranchInst::Create(OnlyDest, Term);
       Term->eraseFromParent();
 
       // If the condition is now dead due to the removal of the old terminator,
       // erase it.
       if (auto *CondInst = dyn_cast<Instruction>(Cond)) {
         if (CondInst->use_empty() && !CondInst->mayHaveSideEffects())
           CondInst->eraseFromParent();
-        // TODO: We can safely replace *some* uses of the CondInst if it has
+        // We can safely replace *some* uses of the CondInst if it has
         // exactly one value as returned by LVI. RAUW is incorrect in the
         // presence of guards and assumes, that have the `Cond` as the use. This
         // is because we use the guards/assume to reason about the `Cond` value
         // at the end of block, but RAUW unconditionally replaces all uses
         // including the guards/assumes themselves and the uses before the
         // guard/assume.
+        else if (OnlyVal && OnlyVal != MultipleVal &&
+                 CondInst->getParent() == BB)
+          ReplaceFoldableUses(CondInst, OnlyVal);
       }
       return true;
     }
   }
 
   // Determine which is the most common successor.  If we have many inputs and
   // this block is a switch, we want to start by threading the batch that goes
   // to the most popular destination first.  If we only know about one

llvm/trunk/lib/Transforms/Utils/Local.cpp

for (Value::use_iterator UI = From->use_begin(), UE = From->use_end();
     U.set(To);
     DEBUG(dbgs() << "Replace dominated use of '" << From->getName() << "' as "
                  << *To << " in " << *U << "\n");
     ++Count;
   }
   return Count;
 }
 
+unsigned llvm::replaceNonLocalUsesWith(Instruction *From, Value *To) {
+   assert(From->getType() == To->getType());
+   auto *BB = From->getParent();
+   unsigned Count = 0;
+
+  for (Value::use_iterator UI = From->use_begin(), UE = From->use_end();
+       UI != UE;) {
+    Use &U = *UI++;
+    auto *I = cast<Instruction>(U.getUser());
+    if (I->getParent() == BB)
+      continue;
+    U.set(To);
+    ++Count;
+  }
+  return Count;
+}
+
 unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To,
                                         DominatorTree &DT,
                                         const BasicBlockEdge &Root) {
   auto Dominates = [&DT](const BasicBlockEdge &Root, const Use &U) {
     return DT.dominates(Root, U);
   };
   return ::replaceDominatedUsesWith(From, To, Root, Dominates);
 }

llvm/trunk/test/Transforms/JumpThreading/assume.ll

 return:                                           ; preds = %entry, %if.then
   %retval.0 = phi i32 [ 1, %if.then ], [ 0, %entry ]
   ret i32 %retval.0
 }
 
 @g = external global i32
 
 ; Check that we do prove a fact using an assume within the block.
-; FIXME: We can fold the assume based on the semantics of assume.
-; CHECK-LABEL: @can_fold_assume
-; CHECK: %notnull = icmp ne i32* %array, null
-; CHECK-NEXT: call void @llvm.assume(i1 %notnull)
-; CHECK-NEXT: ret void
+; We can fold the assume based on the semantics of assume.
 define void @can_fold_assume(i32* %array) {
+; CHECK-LABEL: @can_fold_assume
+; CHECK-NOT: call void @llvm.assume
+; CHECK-NOT: br
+; CHECK: ret void
   %notnull = icmp ne i32* %array, null
   call void @llvm.assume(i1 %notnull)
   br i1 %notnull, label %normal, label %error
 
 normal:
   ret void
 
 error:
   store atomic i32 0, i32* @g unordered, align 4
   ret void
 }
 
 declare void @f(i1)
 declare void @exit()
 ; We can fold the assume but not the uses before the assume.
-define void @dont_fold_incorrectly(i32* %array) {
-; CHECK-LABEL:@dont_fold_incorrectly
+define void @cannot_fold_use_before_assume(i32* %array) {
+; CHECK-LABEL:@cannot_fold_use_before_assume
 ; CHECK: @f(i1 %notnull)
 ; CHECK-NEXT: exit()
-; CHECK-NEXT: assume(i1 %notnull)
+; CHECK-NOT: assume
+; CHECK-NEXT: ret void
+  %notnull = icmp ne i32* %array, null
+  call void @f(i1 %notnull)
+  call void @exit()
+  call void @llvm.assume(i1 %notnull)
+  br i1 %notnull, label %normal, label %error
+
+normal:
+  ret void
+
+error:
+  store atomic i32 0, i32* @g unordered, align 4
+  ret void
+}
+
+declare void @dummy(i1) nounwind argmemonly
+define void @can_fold_some_use_before_assume(i32* %array) {
+
+; CHECK-LABEL:@can_fold_some_use_before_assume
+; CHECK: @f(i1 %notnull)
+; CHECK-NEXT: @dummy(i1 true)
+; CHECK-NOT: assume
 ; CHECK-NEXT: ret void
   %notnull = icmp ne i32* %array, null
   call void @f(i1 %notnull)
+  call void @dummy(i1 %notnull)
+  call void @llvm.assume(i1 %notnull)
+  br i1 %notnull, label %normal, label %error
+
+normal:
+  ret void
+
+error:
+  store atomic i32 0, i32* @g unordered, align 4
+  ret void
+
+}
+
+; FIXME: can fold assume and all uses before/after assume.
+; because the trapping exit call is after the assume.
+define void @can_fold_assume_and_all_uses(i32* %array) {
+; CHECK-LABEL:@can_fold_assume_and_all_uses
+; CHECK: @dummy(i1 %notnull)
+; CHECK-NEXT: assume(i1 %notnull)
+; CHECK-NEXT: exit()
+; CHECK-NEXT: %notnull2 = or i1 true, false
+; CHECK-NEXT: @f(i1 %notnull2)
+; CHECK-NEXT: ret void
+  %notnull = icmp ne i32* %array, null
+  call void @dummy(i1 %notnull)
+  call void @llvm.assume(i1 %notnull)
   call void @exit()
+  br i1 %notnull, label %normal, label %error
+
+normal:
+  %notnull2 = or i1 %notnull, false
+  call void @f(i1 %notnull2)
+  ret void
+
+error:
+  store atomic i32 0, i32* @g unordered, align 4
+  ret void
+}
+
+declare void @fz(i8)
+; FIXME: We can fold assume to true, and the use after assume, but we do not do so
+; currently, because of the function call after the assume.
+define void @can_fold_assume2(i32* %array) {
+
+; CHECK-LABEL:@can_fold_assume2
+; CHECK: @f(i1 %notnull)
+; CHECK-NEXT: assume(i1 %notnull)
+; CHECK-NEXT: znotnull = zext i1 %notnull to i8
+; CHECK-NEXT: @f(i1 %notnull)
+; CHECK-NEXT: @f(i1 true)
+; CHECK-NEXT: @fz(i8 %znotnull)
+; CHECK-NEXT: ret void
+  %notnull = icmp ne i32* %array, null
+  call void @f(i1 %notnull)
+  call void @llvm.assume(i1 %notnull)
+  %znotnull = zext i1 %notnull to i8
+  call void @f(i1 %notnull)
+  br i1 %notnull, label %normal, label %error
+
+normal:
+  call void @f(i1 %notnull)
+  call void @fz(i8 %znotnull)
+  ret void
+
+error:
+  store atomic i32 0, i32* @g unordered, align 4
+  ret void
+}
+
+declare void @llvm.experimental.guard(i1, ...)
+; FIXME: We can fold assume to true, but we do not do so
+; because of the guard following the assume.
+define void @can_fold_assume3(i32* %array){
+
+; CHECK-LABEL:@can_fold_assume3
+; CHECK: @f(i1 %notnull)
+; CHECK-NEXT: assume(i1 %notnull)
+; CHECK-NEXT: guard(i1 %notnull)
+; CHECK-NEXT: znotnull = zext i1 true to i8
+; CHECK-NEXT: @f(i1 true)
+; CHECK-NEXT: @fz(i8 %znotnull)
+; CHECK-NEXT: ret void
+  %notnull = icmp ne i32* %array, null
+  call void @f(i1 %notnull)
   call void @llvm.assume(i1 %notnull)
+  call void(i1, ...) @llvm.experimental.guard(i1 %notnull) [ "deopt"() ]
+  %znotnull = zext i1 %notnull to i8
   br i1 %notnull, label %normal, label %error
 
 normal:
+  call void @f(i1 %notnull)
+  call void @fz(i8 %znotnull)
   ret void
 
 error:
   store atomic i32 0, i32* @g unordered, align 4
   ret void
 }
 
+
+; can fold all uses and remove the cond
+define void @can_fold_assume4(i32* %array) {
+; CHECK-LABEL: can_fold_assume4
+; CHECK-NOT: notnull
+; CHECK: dummy(i1 true)
+; CHECK-NEXT: ret void
+  %notnull = icmp ne i32* %array, null
+  call void @exit()
+  call void @dummy(i1 %notnull)
+  call void @llvm.assume(i1 %notnull)
+  br i1 %notnull, label %normal, label %error
+
+normal:
+  ret void
+
+error:
+  store atomic i32 0, i32* @g unordered, align 4
+  ret void
+}
 ; Function Attrs: nounwind
 declare void @llvm.assume(i1) #1
 
 declare void @bar(...)
 
 declare void @car(...)
 
 attributes #0 = { nounwind uwtable }
 attributes #1 = { nounwind }
 

llvm/trunk/test/Transforms/JumpThreading/fold-not-thread.ll

 L2:
   call void @f3()
   ret void
 L3:
   call void @f3()
   ret void
 }
 
-; FIXME: Make sure we can do the RAUW for %add...
+; Make sure we can do the RAUW for %add...
 ;
 ; CHECK-LABEL: @rauw_if_possible(
-; CHECK: call void @f4(i32 %add)
+; CHECK: call void @f4(i32 96)
 define void @rauw_if_possible(i32 %value) nounwind {
 entry:
   %cmp = icmp eq i32 %value, 32
   br i1 %cmp, label %L0, label %L3 
 L0:
   call i32 @f2()
   call i32 @f2()
   %add = add i32 %value, 64

llvm/trunk/test/Transforms/JumpThreading/guards.ll

 
 Exit:
 ; CHECK:       Merge:
 ; CHECK-NEXT:    call void (i1, ...) @llvm.experimental.guard(i1 %cond1) [ "deopt"() ]
 ; CHECK-NEXT:    ret void
   ret void
 }
 
-declare void @never_called()
+declare void @never_called(i1)
 
-; Assume the guard is always taken and we deoptimize, so we never reach the
-; branch below that guard. We should *never* change the behaviour of a guard from
-; `must deoptimize` to `may deoptimize`, since this affects the program
-; semantics.
+; LVI uses guard to identify value of %c2 in branch as true, we cannot replace that
+; guard with guard(true & c1).
 define void @dont_fold_guard(i8* %addr, i32 %i, i32 %length) {
 ; CHECK-LABEL: dont_fold_guard
-; CHECK: experimental.guard(i1 %wide.chk)
-
-entry:
-  br label %BBPred
+; CHECK: %wide.chk = and i1 %c1, %c2
+; CHECK-NEXT: experimental.guard(i1 %wide.chk)
+; CHECK-NEXT: call void @never_called(i1 true)
+; CHECK-NEXT: ret void
+  %c1 = icmp ult i32 %i, %length
+  %c2 = icmp eq i32 %i, 0
+  %wide.chk = and i1 %c1, %c2
+  call void(i1, ...) @llvm.experimental.guard(i1 %wide.chk) [ "deopt"() ]
+  br i1 %c2, label %BB1, label %BB2
 
-BBPred:
- %cond = icmp eq i8* %addr, null
- br i1 %cond, label %zero, label %not_zero
+BB1:
+  call void @never_called(i1 %c2)
+  ret void
 
-zero:
-  unreachable
+BB2:
+  ret void
+}
 
-not_zero:
+declare void @dummy(i1) nounwind argmemonly
+; same as dont_fold_guard1 but there's a use immediately after guard and before
+; branch. We can fold that use.
+define void @dont_fold_guard2(i8* %addr, i32 %i, i32 %length) {
+; CHECK-LABEL: dont_fold_guard2
+; CHECK: %wide.chk = and i1 %c1, %c2
+; CHECK-NEXT: experimental.guard(i1 %wide.chk)
+; CHECK-NEXT: dummy(i1 true)
+; CHECK-NEXT: call void @never_called(i1 true)
+; CHECK-NEXT: ret void
   %c1 = icmp ult i32 %i, %length
   %c2 = icmp eq i32 %i, 0
   %wide.chk = and i1 %c1, %c2
   call void(i1, ...) @llvm.experimental.guard(i1 %wide.chk) [ "deopt"() ]
-  br i1 %c2, label %unreachedBB2, label %unreachedBB1
+  call void @dummy(i1 %c2)
+  br i1 %c2, label %BB1, label %BB2
 
-unreachedBB2:
-  call void @never_called()
+BB1:
+  call void @never_called(i1 %c2)
   ret void
 
-unreachedBB1:
+BB2:
   ret void
 }
 
-
 ; same as dont_fold_guard1 but condition %cmp is not an instruction.
 ; We cannot fold the guard under any circumstance.
 ; FIXME: We can merge unreachableBB2 into not_zero.
-define void @dont_fold_guard2(i8* %addr, i1 %cmp, i32 %i, i32 %length) {
-; CHECK-LABEL: dont_fold_guard2
+define void @dont_fold_guard3(i8* %addr, i1 %cmp, i32 %i, i32 %length) {
+; CHECK-LABEL: dont_fold_guard3
 ; CHECK: guard(i1 %cmp)
-
-entry:
-  br label %BBPred
-
-BBPred:
- %cond = icmp eq i8* %addr, null
- br i1 %cond, label %zero, label %not_zero
-
-zero:
-  unreachable
-
-not_zero:
   call void(i1, ...) @llvm.experimental.guard(i1 %cmp) [ "deopt"() ]
-  br i1 %cmp, label %unreachedBB2, label %unreachedBB1
+  br i1 %cmp, label %BB1, label %BB2
 
-unreachedBB2:
-  call void @never_called()
+BB1:
+  call void @never_called(i1 %cmp)
   ret void
 
-unreachedBB1:
+BB2:
   ret void
 }
 
+declare void @f(i1)
 ; Same as dont_fold_guard1 but use switch instead of branch.
 ; triggers source code `ProcessThreadableEdges`.
-declare void @f(i1)
-define void @dont_fold_guard3(i1 %cmp1, i32 %i) nounwind {
-; CHECK-LABEL: dont_fold_guard3 
+define void @dont_fold_guard4(i1 %cmp1, i32 %i) nounwind {
+; CHECK-LABEL: dont_fold_guard4 
 ; CHECK-LABEL: L2:
 ; CHECK-NEXT: %cmp = icmp eq i32 %i, 0 
 ; CHECK-NEXT: guard(i1 %cmp)
-; CHECK-NEXT: @f(i1 %cmp)
+; CHECK-NEXT: dummy(i1 true)
+; CHECK-NEXT: @f(i1 true)
 ; CHECK-NEXT: ret void
 entry:
   br i1 %cmp1, label %L0, label %L3 
 L0:
   %cmp = icmp eq i32 %i, 0
   call void(i1, ...) @llvm.experimental.guard(i1 %cmp) [ "deopt"() ]
+  call void @dummy(i1 %cmp)
   switch i1 %cmp, label %L3 [
     i1 false, label %L1
     i1 true, label %L2
     ]
 
 L1:
   ret void
 L2:
   call void @f(i1 %cmp)
   ret void
 L3:
   ret void
 }