[SimpleLoopUnswitch] Re-fix introduction of UB when hoisted condition may be undef or poison

[hyeongyukim]

Summary

https://bugs.llvm.org/show_bug.cgi?id=27506
https://bugs.llvm.org/show_bug.cgi?id=31652
https://bugs.llvm.org/show_bug.cgi?id=51043

Problems with SimpleLoopUnswitch cause the bug reports above.

while (...) {
  if (C) { A }
  else   { B }
}
Into:

C' = freeze(C)
if (C') {
  while (...) { A }
} else {
  while (...) { B }
}

This problem can be solved by adding a freeze on hoisted branches(above transform) and has been solved by D29015.
However, D29015 is now reverted by performance regression(https://github.com/llvm/llvm-project/commit/2b5a8976514de326bb84f0913d9d451089c11d22)

It is not the first time that an added freeze has caused performance regression.
SimplifyCFG also had a problem with UB caused by branching-on-undef, which was solved by adding freeze to the branching condition. (D104569)
Performance regression occurred in D104569, and patches such as D105344 and D105392 were written to minimize it.

This patch will correct the SimpleLoopUnswitch as D104569 handles the SimplyCFG while minimizing performance loss by introducing patches like D105344 and D105392(This patch was rebased with the author's permission)

[hyeongyukim]

When this patch was applied, there is a significant change in the compiled assembly of LLVM testsuite.
There have been changes to 123 of the 1977 files, and I am now looking for ways to reduce them.

[hyeongyukim]

While analyzing the cause of performance regression, I found that freeze(icmp(..)) is interrupting SimplifyCFG.
And most of them will be solved by pushing freeze to icmp's operands. (Make freeze(icmp(Op0, Op1)) to icmp(freeze(Op0), freeze(Op1)))

if (InsertFreeze) {
  auto Cond = SI->getCondition();
  if (!isGuaranteedNotToBeUndefOrPoison(Cond, &AC, BI, &DT)) {
    if (ICmpInst* I = dyn_cast<ICmpInst>(Cond)) {
      Value* Op0 = I->getOperand(0);
      Value* Op1 = I->getOperand(1);

      auto FreezeOperand = [&](Value* Op) {
        if (isGuaranteedNotToBePoison(Op, &AC, BI, &DT)) 
          return;

        auto *FrozenOp = new FreezeInst(Op, Op->getName() + ".fr");
        FrozenOp->insertBefore(I);
        Op = FrozenOp;
        return;
      };

      FreezeOperand(Op0);
      FreezeOperand(Op1);
    }
    else
      SI->setCondition(new FreezeInst(Cond, Cond->getName() + ".fr", SI));
  }
}

Actually, performance regression was significantly reduced when the above code was added.
Before I push freeze to icmp's operands, 104 assembly diffs were found while compiling the LLVM testsuite.
But after I apply the above code, 104 assembly diffs were reduced to 37. (Which means performance regression was reduced)

But I'm not sure it's the right approach to treat icmp separately inside the LU.
Can you check if it's okay to add these codes? @aqjune

[aqjune]

I think so? it sounds great.
Could you try inserting freeze to icmp only when one of its operands is constant?
ex) freeze (icmp op0, 1) -> icmp (freeze(op0), 1)
ex2) freeze (icmp op0, op1) -/-> icmp (freeze(op0), freeze(op1)) <= this splits one freeze into two; its benefit is not clear.

[lebedev.ri]

Much like with pushing freeze within simplifycfg, doing that in loopunswitch seems misplaced.
It's impossible to tell without a phase-ordering test (one with -O3, not just a single pass),
but perhaps we simply need to run/move instcombine invocation somewhere in the pipeline?

[hyeongyukim]

In D106041#2922186, @lebedev.ri wrote:

Much like with pushing freeze within simplifycfg, doing that in loopunswitch seems misplaced.
It's impossible to tell without a phase-ordering test (one with -O3, not just a single pass),
but perhaps we simply need to run/move instcombine invocation somewhere in the pipeline?

@lebedev.ri Sounds good to me.
Could you please tell me how to change the pipeline?
And... is there anything I should consider while changing the pipeline?

[lebedev.ri]

I'd need to see the test to guess where we need to add it.
Generally, simpleloopunswitch is only run once,
scheduled in PassBuilder::buildFunctionSimplificationPipeline() (line 761),

LPM1.addPass(
    SimpleLoopUnswitchPass(/* NonTrivial */ Level == OptimizationLevel::O3 &&
                           EnableO3NonTrivialUnswitching));

While the very next simplifycfg (line 796) runs before the first instcombine run (line 797),
there's further simplifycfg run (line 858) that runs after instcombine.
So i'm not sure what's going on, and i won't be able to help without a test...

[hyeongyukim]

Oh, thank you for letting me know.
I'll give it a try!

[hyeongyukim]

If Cond is not one-use, pushFreezeToPreventPoisonFromPropagating is not working, so the Freeze cannot be pushed.
Unpushed freezes are disturbing SimplyCFG, so I changed all uses of Cond to use Cond.fr.

[hyeongyukim]

We talked about changing the pipeline, but there is no need to change the pipeline.
And the performance degradation caused by this patch was also reduced (104 differences were reduced to 99.) @aqjune

[hyeongyukim]

While checking the performance regression caused by this patch, I found the following issue:

  Cond.fr = freeze(Cond)
  br Cond.fr, BB_True, BB_False
BB_True:
  use(Cond.fr)			; Cond.fr is not replaced to true
  ...
BB_False:
  use(Cond.fr)			; Cond.fr is not repalced to false
  ...

In Original LoopUnswitch, we check the use of invariants and replace them if they are dominated by LoopPH or ClonedPH.
But while fixing LoopUnswitch, I inserted freeze to Cond(which is invariant) and replaced all original uses of Cond to Cond.fr.
Not only Cond but also Cond.fr is invariant, but the current implementation doesn't know that Cond.fr is invariant.
I added a code and test code(test1_freeze) that tells that Cond and Cond.fr are both invariants.

[hyeongyukim]

Add Freeze-loop-unswitch-cond flag.
This flag determines whether to add a freeze to the condition of the loop switch.
When this flag is turned on, the miscompilation is solved, but It will degrade the performance.
The default value of the flag is set to false until the performance problem is completely solved.

[hyeongyukim]

update test codes

[hyeongyukim]

refix test code

[hyeongyukim]

Since the flag that adds the freeze is off, there is no change in the existing test code.
How about adding -freeze-loop-unswitch-cond to some of the tests so that we can see where the freeze is added?

[hyeongyukim]

Add nontrivial-unswitch-freeze.ll to show changes.

[hyeongyukim]

remove unused sink functions.

[xbolva00]

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

2221

Consider new helper function

[reames]

See inline review comments. Overall intent of my suggestions is to cut away everything needing further discussion so that we can get the simplest patch in tree (off by default) and then revisit pieces needing discussion in separate reviews.

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

2206

This block of code appears to be re-implementing LICM, and it's not clear we should do this here at all. Please refresh the patch to place the freeze unconditionally before the branch, and then revisit this piece in a separate review with it's own justification.

2221

If we keep the code, agreed. See also comment on previous copy of code, also applies here.

2412

The use of the lambda here is NFC, the code change is reasonable on it's own (without the addition), please pull out and submit separately (without additional review) to simplify this patch.

2414

Do you mind moving this to a separate patch? I think we can do better than this. Specifically, for the case where V is a constant, *many* instructions constant fold and thus we could possibly do something for all users which happens to constant fold freeze without special magic.

llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-freeze.ll

1 ↗

(On Diff #377469)

Please use ./utils/update_test_checks.py

[hyeongyukim]

Right, many codes were not directly related to solving the loop unswitch problem.
Those codes were created to reduce performance regression, and I agree to separate them into different patches.

[hyeongyukim]

Changed to the original patch.

[reames]

LGTM

We can do better about caching and reusing the expensive analyzes here (ICF, and guaranteed-non-poison), but that can be done in following changes given this is off-by-default.

[hyeongyukim]

Thank you for the review.
I will also prepare for improvement!

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

 #include "llvm/Analysis/LoopAnalysisManager.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/LoopIterator.h"
 #include "llvm/Analysis/LoopPass.h"
 #include "llvm/Analysis/MemorySSA.h"
 #include "llvm/Analysis/MemorySSAUpdater.h"
 #include "llvm/Analysis/MustExecute.h"
 #include "llvm/Analysis/ScalarEvolution.h"
+#include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
if (!L.isLoopInvariant(PN->getIncomingValueForBlock(&ExitingBB)))
       return false;
   }
   llvm_unreachable("Basic blocks should never be empty!");
 }
 
 /// Copy a set of loop invariant values \p ToDuplicate and insert them at the
 /// end of \p BB and conditionally branch on the copied condition. We only
 /// branch on a single value.
 static void buildPartialUnswitchConditionalBranch(BasicBlock &BB,
                                                   ArrayRef<Value *> Invariants,
                                                   bool Direction,
                                                   BasicBlock &UnswitchedSucc,
-                                                  BasicBlock &NormalSucc) {
+                                                  BasicBlock &NormalSucc,
+                                                  bool InsertFreeze) {
   IRBuilder<> IRB(&BB);
 
   Value *Cond = Direction ? IRB.CreateOr(Invariants) :
     IRB.CreateAnd(Invariants);
+  if (InsertFreeze)
+    Cond = IRB.CreateFreeze(Cond, Cond->getName() + ".fr");
   IRB.CreateCondBr(Cond, Direction ? &UnswitchedSucc : &NormalSucc,
                    Direction ? &NormalSucc : &UnswitchedSucc);
 }
 
 /// Copy a set of loop invariant values, and conditionally branch on them.
 static void buildPartialInvariantUnswitchConditionalBranch(
     BasicBlock &BB, ArrayRef<Value *> ToDuplicate, bool Direction,
     BasicBlock &UnswitchedSucc, BasicBlock &NormalSucc, Loop &L,
if (ExitDirection)
       assert(match(BI.getCondition(), m_LogicalOr()) &&
              "Must have an `or` of `i1`s or `select i1 X, true, Y`s for the "
              "condition!");
     else
       assert(match(BI.getCondition(), m_LogicalAnd()) &&
              "Must have an `and` of `i1`s or `select i1 X, Y, false`s for the"
              " condition!");
     buildPartialUnswitchConditionalBranch(*OldPH, Invariants, ExitDirection,
-                                          *UnswitchedBB, *NewPH);
+                                          *UnswitchedBB, *NewPH, false);
   }
 
   // Update the dominator tree with the added edge.
   DT.insertEdge(OldPH, UnswitchedBB);
 
   // After the dominator tree was updated with the added edge, update MemorySSA
   // if available.
   if (MSSAU) {
static void unswitchNontrivialInvariants(
   // containing an exit block and all nested loops.
   if (SE) {
     if (OuterExitL)
       SE->forgetLoop(OuterExitL);
     else
       SE->forgetTopmostLoop(&L);
   }
 
+  ICFLoopSafetyInfo SafetyInfo;
+  SafetyInfo.computeLoopSafetyInfo(&L);
+  bool InsertFreeze = !SafetyInfo.isGuaranteedToExecute(TI, &DT, &L);
+
   // If the edge from this terminator to a successor dominates that successor,
   // store a map from each block in its dominator subtree to it. This lets us
   // tell when cloning for a particular successor if a block is dominated by
   // some *other* successor with a single data structure. We use this to
   // significantly reduce cloning.
   SmallDenseMap<BasicBlock *, BasicBlock *, 16> DominatingSucc;
   for (auto *SuccBB : llvm::concat<BasicBlock *const>(
            makeArrayRef(RetainedSuccBB), UnswitchedSuccBBs))
if (FullUnswitch) {
     Instruction *NewTI = TI.clone();
     ParentBB->getInstList().push_back(NewTI);
 
     // First wire up the moved terminator to the preheaders.
     if (BI) {
       BasicBlock *ClonedPH = ClonedPHs.begin()->second;
       BI->setSuccessor(ClonedSucc, ClonedPH);
       BI->setSuccessor(1 - ClonedSucc, LoopPH);
+      if (InsertFreeze) {
+        auto Cond = BI->getCondition();
+        if (!isGuaranteedNotToBeUndefOrPoison(Cond, &AC, BI, &DT))
+          BI->setCondition(new FreezeInst(Cond, Cond->getName() + ".fr", BI));
+      }
       DTUpdates.push_back({DominatorTree::Insert, SplitBB, ClonedPH});
     } else {
       assert(SI && "Must either be a branch or switch!");
 
       // Walk the cases and directly update their successors.
       assert(SI->getDefaultDest() == RetainedSuccBB &&
              "Not retaining default successor!");
       SI->setDefaultDest(LoopPH);
       for (auto &Case : SI->cases())
         if (Case.getCaseSuccessor() == RetainedSuccBB)
           Case.setSuccessor(LoopPH);
         else
           Case.setSuccessor(ClonedPHs.find(Case.getCaseSuccessor())->second);
 
+      if (InsertFreeze) {
+        auto Cond = SI->getCondition();
+        if (!isGuaranteedNotToBeUndefOrPoison(Cond, &AC, SI, &DT))
+          SI->setCondition(new FreezeInst(Cond, Cond->getName() + ".fr", SI));
+      }
       // We need to use the set to populate domtree updates as even when there
       // are multiple cases pointing at the same successor we only want to
       // remove and insert one edge in the domtree.
       for (BasicBlock *SuccBB : UnswitchedSuccBBs)
         DTUpdates.push_back(
             {DominatorTree::Insert, SplitBB, ClonedPHs.find(SuccBB)->second});
     }
 
if (FullUnswitch) {
     BasicBlock *ClonedPH = ClonedPHs.begin()->second;
     // When doing a partial unswitch, we have to do a bit more work to build up
     // the branch in the split block.
     if (PartiallyInvariant)
       buildPartialInvariantUnswitchConditionalBranch(
           *SplitBB, Invariants, Direction, *ClonedPH, *LoopPH, L, MSSAU);
     else
       buildPartialUnswitchConditionalBranch(*SplitBB, Invariants, Direction,
-                                            *ClonedPH, *LoopPH);
+                                            *ClonedPH, *LoopPH, InsertFreeze);
     DTUpdates.push_back({DominatorTree::Insert, SplitBB, ClonedPH});
 
     if (MSSAU) {
       DT.applyUpdates(DTUpdates);
       DTUpdates.clear();
 
       // Perform MSSA cloning updates.
       for (auto &VMap : VMaps)
static void unswitchNontrivialInvariants(
   // with the current loop. As a consequence, we need to re-form LCSSA for
   // them. But we shouldn't need to re-form LCSSA for any child loops.
   // FIXME: This could be made more efficient by tracking which exit blocks are
   // new, and focusing on them, but that isn't likely to be necessary.
   //
   // In order to reasonably rebuild LCSSA we need to walk inside-out across the
   // loop nest and update every loop that could have had its exits changed. We
   // also need to cover any intervening loops. We add all of these loops to
   // a list and sort them by loop depth to achieve this without updating
   // unnecessary loops.
   auto UpdateLoop = [&](Loop &UpdateL) {
 #ifndef NDEBUG
     UpdateL.verifyLoop();
     for (Loop *ChildL : UpdateL) {
       ChildL->verifyLoop();
       assert(ChildL->isRecursivelyLCSSAForm(DT, LI) &&
              "Perturbed a child loop's LCSSA form!");
     }
 #endif

llvm/test/Transforms/SimpleLoopUnswitch/ARM/nontrivial-unswitch-cost.ll

 
 loop_exit:
   ret void
 }
 
 define void @test_unswitch(i1* %ptr, i1 %cond) {
 ; CHECK-LABEL: @test_unswitch(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 [[COND:%.*]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
+; CHECK-NEXT:    [[COND_FR:%.*]] = freeze i1 [[COND:%.*]]
+; CHECK-NEXT:    br i1 [[COND_FR]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
 ; CHECK:       entry.split.us:
 ; CHECK-NEXT:    br label [[LOOP_BEGIN_US:%.*]]
 ; CHECK:       loop_begin.us:
 ; CHECK-NEXT:    call void @x()
 ; CHECK-NEXT:    br label [[LOOP_A_US:%.*]]
 ; CHECK:       loop_a.us:
 ; CHECK-NEXT:    call void @a()
 ; CHECK-NEXT:    br label [[LOOP_LATCH_US:%.*]]

 
 loop_exit:
   ret void
 }
 
 define void @test_unswitch_non_dup_code(i1* %ptr, i1 %cond) {
 ; CHECK-LABEL: @test_unswitch_non_dup_code(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 [[COND:%.*]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
+; CHECK-NEXT:    [[COND_FR:%.*]] = freeze i1 [[COND:%.*]]
+; CHECK-NEXT:    br i1 [[COND_FR]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
 ; CHECK:       entry.split.us:
 ; CHECK-NEXT:    br label [[LOOP_BEGIN_US:%.*]]
 ; CHECK:       loop_begin.us:
 ; CHECK-NEXT:    call void @x()
 ; CHECK-NEXT:    br label [[LOOP_A_US:%.*]]
 ; CHECK:       loop_a.us:
 ; CHECK-NEXT:    call void @a()
 ; CHECK-NEXT:    call void @a()

 
 loop_exit:
   ret void
 }
 
 define void @test_unswitch_non_dup_code_in_cfg(i1* %ptr, i1 %cond) {
 ; CHECK-LABEL: @test_unswitch_non_dup_code_in_cfg(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 [[COND:%.*]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
+; CHECK-NEXT:    [[COND_FR:%.*]] = freeze i1 [[COND:%.*]]
+; CHECK-NEXT:    br i1 [[COND_FR]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
 ; CHECK:       entry.split.us:
 ; CHECK-NEXT:    br label [[LOOP_BEGIN_US:%.*]]
 ; CHECK:       loop_begin.us:
 ; CHECK-NEXT:    call void @x()
 ; CHECK-NEXT:    br label [[LOOP_A_US:%.*]]
 ; CHECK:       loop_a.us:
 ; CHECK-NEXT:    [[V1_US:%.*]] = load i1, i1* [[PTR:%.*]], align 1
 ; CHECK-NEXT:    br i1 [[V1_US]], label [[LOOP_A_A_US:%.*]], label [[LOOP_A_B_US:%.*]]

 
 loop_exit:
   ret void
 }
 
 define void @test_unswitch_large_exit(i1* %ptr, i1 %cond) {
 ; CHECK-LABEL: @test_unswitch_large_exit(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 [[COND:%.*]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
+; CHECK-NEXT:    [[COND_FR:%.*]] = freeze i1 [[COND:%.*]]
+; CHECK-NEXT:    br i1 [[COND_FR]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
 ; CHECK:       entry.split.us:
 ; CHECK-NEXT:    br label [[LOOP_BEGIN_US:%.*]]
 ; CHECK:       loop_begin.us:
 ; CHECK-NEXT:    call void @x()
 ; CHECK-NEXT:    br label [[LOOP_A_US:%.*]]
 ; CHECK:       loop_a.us:
 ; CHECK-NEXT:    call void @a()
 ; CHECK-NEXT:    br label [[LOOP_LATCH_US:%.*]]
loop_exit:
   call void @x()
   call void @x()
   ret void
 }
 
 define void @test_unswitch_dedicated_exiting(i1* %ptr, i1 %cond) {
 ; CHECK-LABEL: @test_unswitch_dedicated_exiting(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 [[COND:%.*]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
+; CHECK-NEXT:    [[COND_FR:%.*]] = freeze i1 [[COND:%.*]]
+; CHECK-NEXT:    br i1 [[COND_FR]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
 ; CHECK:       entry.split.us:
 ; CHECK-NEXT:    br label [[LOOP_BEGIN_US:%.*]]
 ; CHECK:       loop_begin.us:
 ; CHECK-NEXT:    call void @x()
 ; CHECK-NEXT:    br label [[LOOP_A_US:%.*]]
 ; CHECK:       loop_a.us:
 ; CHECK-NEXT:    call void @a()
 ; CHECK-NEXT:    br label [[LOOP_LATCH_US:%.*]]

llvm/test/Transforms/SimpleLoopUnswitch/implicit-null-checks.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -enable-nontrivial-unswitch=true -simple-loop-unswitch -S < %s | FileCheck %s
 ; RUN: opt -enable-nontrivial-unswitch=true -passes='loop(simple-loop-unswitch),verify<loops>' -S < %s | FileCheck %s
 
 declare void @may_exit()
 declare void @throw_npe()
 
 ; It is illegal to preserve make_implicit notion of the condition being
 ; unswitched because we may exit loop before we reach the condition, so
 ; there is no guarantee that following implicit branch always means getting
 ; to throw_npe block.
 define i32 @test_should_drop_make_implicit(i32* %p1, i32* %p2) {
 ; CHECK-LABEL: @test_should_drop_make_implicit(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[NULL_CHECK:%.*]] = icmp eq i32* [[P2:%.*]], null
-; CHECK-NOT:     !make.implicit
-; CHECK-NEXT:    br i1 [[NULL_CHECK]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
+; CHECK-NEXT:    [[NULL_CHECK_FR:%.*]] = freeze i1 [[NULL_CHECK]]
+; CHECK-NEXT:    br i1 [[NULL_CHECK_FR]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
 ; CHECK:       entry.split.us:
 ; CHECK-NEXT:    br label [[LOOP_US:%.*]]
 ; CHECK:       loop.us:
 ; CHECK-NEXT:    [[IV_US:%.*]] = phi i32 [ 0, [[ENTRY_SPLIT_US]] ]
 ; CHECK-NEXT:    [[X_US:%.*]] = load i32, i32* [[P1:%.*]], align 4
 ; CHECK-NEXT:    [[SIDE_EXIT_COND_US:%.*]] = icmp eq i32 [[X_US]], 0
 ; CHECK-NEXT:    br i1 [[SIDE_EXIT_COND_US]], label [[SIDE_EXIT_SPLIT_US:%.*]], label [[NULL_CHECK_BLOCK_US:%.*]]
 ; CHECK:       null_check_block.us:
exit:
   ret i32 %x
 }
 
 ; Here make.implicit notion should be dropped because of exiting call.
 define i32 @test_should_drop_make_implicit_exiting_call(i32* %p1, i32* %p2) {
 ; CHECK-LABEL: @test_should_drop_make_implicit_exiting_call(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[NULL_CHECK:%.*]] = icmp eq i32* [[P2:%.*]], null
-; CHECK-NOT:     !make.implicit
-; CHECK-NEXT:    br i1 [[NULL_CHECK]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
+; CHECK-NEXT:    [[NULL_CHECK_FR:%.*]] = freeze i1 [[NULL_CHECK]]
+; CHECK-NEXT:    br i1 [[NULL_CHECK_FR]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
 ; CHECK:       entry.split.us:
 ; CHECK-NEXT:    br label [[LOOP_US:%.*]]
 ; CHECK:       loop.us:
 ; CHECK-NEXT:    [[IV_US:%.*]] = phi i32 [ 0, [[ENTRY_SPLIT_US]] ]
 ; CHECK-NEXT:    call void @may_exit()
 ; CHECK-NEXT:    [[X_US:%.*]] = load i32, i32* [[P1:%.*]], align 4
 ; CHECK-NEXT:    [[SIDE_EXIT_COND_US:%.*]] = icmp eq i32 [[X_US]], 0
 ; CHECK-NEXT:    br label [[THROW_NPE_SPLIT_US:%.*]]