[InstCombine] Fix non-convergence with freeze/phi folding by rahulana-quic · Pull Request #182647 · llvm/llvm-project

rahulana-quic · 2026-02-21T02:50:27Z

InstCombine can fail to reach a fixpoint due to oscillation between
freeze(phi(...)) and phi(..., freeze(x), ...) when the newly introduced
incoming freezes are immediately simplified away again.

Add a guard in foldOpIntoPhi() to avoid sinking a FreezeInst into a phi
when an incoming freeze would simplify to the incoming value. Also treat
FreezeInst as a boundary for pushFreezeToPreventPoisonFromPropagating()
to avoid stacking up freezes when pushing them.

fixes #182475

InstCombine can fail to reach a fixpoint due to oscillation between freeze(phi(...)) and phi(..., freeze(x), ...) when the newly introduced incoming freezes are immediately simplified away again. Add a guard in foldOpIntoPhi() to avoid sinking a FreezeInst into a phi when an incoming freeze would simplify to the incoming value. Also treat FreezeInst as a boundary for pushFreezeToPreventPoisonFromPropagating() to avoid stacking up freezes when pushing them.

llvmbot · 2026-02-21T02:50:58Z

@llvm/pr-subscribers-llvm-transforms

Author: None (rahulana-quic)

Changes

InstCombine can fail to reach a fixpoint due to oscillation between
freeze(phi(...)) and phi(..., freeze(x), ...) when the newly introduced
incoming freezes are immediately simplified away again.

Add a guard in foldOpIntoPhi() to avoid sinking a FreezeInst into a phi
when an incoming freeze would simplify to the incoming value. Also treat
FreezeInst as a boundary for pushFreezeToPreventPoisonFromPropagating()
to avoid stacking up freezes when pushing them.

Full diff: https://github.com/llvm/llvm-project/pull/182647.diff

2 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+11)
(modified) llvm/test/Transforms/InstCombine/freeze-phi.ll (+40-8)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index c4beacdd12684..3a44253f8ecb1 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1980,6 +1980,12 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN,
     Value *InVal = PN->getIncomingValue(i);
     BasicBlock *InBB = PN->getIncomingBlock(i);
 
+    // Avoid repeatedly sinking a freeze into a PHI only for some of the newly
+    // created incoming freezes to be trivially removable again. That pattern
+    // can cause oscillation between freeze(phi(...)) and phi(..., freeze(x), ...).
+    if (isa<FreezeInst>(&I) && isGuaranteedNotToBeUndefOrPoison(InVal))
+      return nullptr;
+
     if (auto *NewVal = simplifyInstructionWithPHI(I, PN, InVal, InBB, DL, SQ)) {
       NewPhiValues.push_back(NewVal);
       continue;
@@ -5214,6 +5220,11 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
     if (!isa<Instruction>(V) || isa<PHINode>(V))
       return false;
 
+    // Don't push through an existing freeze. Otherwise we can repeatedly
+    // create redundant freeze instructions during instcombine iterations.
+    if (isa<FreezeInst>(V))
+      return false;
+
     // We can't push the freeze through an instruction which can itself create
     // poison.  If the only source of new poison is flags, we can simply
     // strip them (since we know the only use is the freeze and nothing can
diff --git a/llvm/test/Transforms/InstCombine/freeze-phi.ll b/llvm/test/Transforms/InstCombine/freeze-phi.ll
index 62bb9dc31b76b..bfec8fba3b22e 100644
--- a/llvm/test/Transforms/InstCombine/freeze-phi.ll
+++ b/llvm/test/Transforms/InstCombine/freeze-phi.ll
@@ -23,6 +23,35 @@ C:
   ret i32 %y.fr
 }
 
+define i32 @trivial_incoming(i1 %cond, i32 %x) {
+; CHECK-LABEL: @trivial_incoming(
+; CHECK-NEXT:    br i1 [[COND:%.*]], label [[A:%.*]], label [[B:%.*]]
+; CHECK:       A:
+; CHECK-NEXT:    br label [[C:%.*]]
+; CHECK:       B:
+; CHECK-NEXT:    br label [[C]]
+; CHECK:       C:
+; CHECK-NEXT:    [[P:%.*]] = phi i32 [ 52, [[A]] ], [ [[X:%.*]], [[B]] ]
+; CHECK-NEXT:    [[FR:%.*]] = freeze i32 [[P]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[FR]], 1
+; CHECK-NEXT:    [[DEC:%.*]] = add nsw i32 [[FR]], -1
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[CMP]], i32 54, i32 [[DEC]]
+; CHECK-NEXT:    ret i32 [[SEL]]
+;
+  br i1 %cond, label %A, label %B
+A:
+  br label %C
+B:
+  br label %C
+C:
+  %p = phi i32 [ 52, %A ], [ %x, %B ]
+  %fr = freeze i32 %p
+  %cmp = icmp slt i32 %fr, 1
+  %dec = add nsw i32 %fr, -1
+  %sel = select i1 %cmp, i32 54, i32 %dec
+  ret i32 %sel
+}
+
 define <2 x i32> @vec(i1 %cond) {
 ; CHECK-LABEL: @vec(
 ; CHECK-NEXT:    br i1 [[COND:%.*]], label [[A:%.*]], label [[B:%.*]]
@@ -53,7 +82,8 @@ define <2 x i32> @vec_undef(i1 %cond) {
 ; CHECK:       B:
 ; CHECK-NEXT:    br label [[C]]
 ; CHECK:       C:
-; CHECK-NEXT:    [[Y:%.*]] = phi <2 x i32> [ <i32 0, i32 1>, [[A]] ], [ splat (i32 2), [[B]] ]
+; CHECK-NEXT:    [[Y1:%.*]] = phi <2 x i32> [ <i32 0, i32 1>, [[A]] ], [ <i32 2, i32 undef>, [[B]] ]
+; CHECK-NEXT:    [[Y:%.*]] = freeze <2 x i32> [[Y1]]
 ; CHECK-NEXT:    ret <2 x i32> [[Y]]
 ;
   br i1 %cond, label %A, label %B
@@ -73,11 +103,11 @@ define i32 @one(i1 %cond, i32 %x) {
 ; CHECK:       A:
 ; CHECK-NEXT:    br label [[C:%.*]]
 ; CHECK:       B:
-; CHECK-NEXT:    [[TMP1:%.*]] = freeze i32 [[X:%.*]]
 ; CHECK-NEXT:    br label [[C]]
 ; CHECK:       C:
-; CHECK-NEXT:    [[Y:%.*]] = phi i32 [ 0, [[A]] ], [ [[TMP1]], [[B]] ]
-; CHECK-NEXT:    ret i32 [[Y]]
+; CHECK-NEXT:    [[Y:%.*]] = phi i32 [ 0, [[A]] ], [ [[TMP1:%.*]], [[B]] ]
+; CHECK-NEXT:    [[Y_FR:%.*]] = freeze i32 [[Y]]
+; CHECK-NEXT:    ret i32 [[Y_FR]]
 ;
   br i1 %cond, label %A, label %B
 A:
@@ -159,7 +189,8 @@ define i32 @one_undef(i8 %cond) {
 ; CHECK:       C:
 ; CHECK-NEXT:    br label [[D]]
 ; CHECK:       D:
-; CHECK-NEXT:    [[Y:%.*]] = phi i32 [ 0, [[A]] ], [ 32, [[B]] ], [ 0, [[C]] ]
+; CHECK-NEXT:    [[Y1:%.*]] = phi i32 [ undef, [[A]] ], [ 32, [[B]] ], [ 0, [[C]] ]
+; CHECK-NEXT:    [[Y:%.*]] = freeze i32 [[Y1]]
 ; CHECK-NEXT:    ret i32 [[Y]]
 ;
   switch i8 %cond, label %A [
@@ -187,14 +218,14 @@ define i32 @one_constexpr(i8 %cond, i32 %x) {
 ; CHECK-NEXT:      i8 1, label [[C:%.*]]
 ; CHECK-NEXT:    ]
 ; CHECK:       A:
-; CHECK-NEXT:    [[TMP1:%.*]] = freeze i32 ptrtoint (ptr getelementptr inbounds nuw (i8, ptr @glb, i64 2) to i32)
 ; CHECK-NEXT:    br label [[D:%.*]]
 ; CHECK:       B:
 ; CHECK-NEXT:    br label [[D]]
 ; CHECK:       C:
 ; CHECK-NEXT:    br label [[D]]
 ; CHECK:       D:
-; CHECK-NEXT:    [[Y:%.*]] = phi i32 [ [[TMP1]], [[A]] ], [ 32, [[B]] ], [ 0, [[C]] ]
+; CHECK-NEXT:    [[Y1:%.*]] = phi i32 [ ptrtoint (ptr getelementptr inbounds nuw (i8, ptr @glb, i64 2) to i32), [[A]] ], [ 32, [[B]] ], [ 0, [[C]] ]
+; CHECK-NEXT:    [[Y:%.*]] = freeze i32 [[Y1]]
 ; CHECK-NEXT:    ret i32 [[Y]]
 ;
   switch i8 %cond, label %A [
@@ -224,7 +255,8 @@ define float @pr161524(float noundef %arg) {
 ; CHECK-NEXT:    [[FADD:%.*]] = fadd float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    br label [[IF_EXIT]]
 ; CHECK:       if.exit:
-; CHECK-NEXT:    [[RET:%.*]] = phi float [ [[FADD]], [[IF_THEN]] ], [ [[ARG]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[RET1:%.*]] = phi ninf float [ [[FADD]], [[IF_THEN]] ], [ [[ARG]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[RET:%.*]] = freeze float [[RET1]]
 ; CHECK-NEXT:    ret float [[RET]]
 ;
 entry:

github-actions · 2026-02-21T02:52:03Z

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:

git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef([^a-zA-Z0-9_-]|$)|UndefValue::get)' 'HEAD~1' HEAD llvm/lib/Transforms/InstCombine/InstructionCombining.cpp llvm/test/Transforms/InstCombine/freeze-phi.ll

The following files introduce new uses of undef:

llvm/test/Transforms/InstCombine/freeze-phi.ll

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.

github-actions · 2026-02-21T02:52:03Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions cpp -- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 3a44253f8..a8e98a451 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1982,7 +1982,8 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN,
 
     // Avoid repeatedly sinking a freeze into a PHI only for some of the newly
     // created incoming freezes to be trivially removable again. That pattern
-    // can cause oscillation between freeze(phi(...)) and phi(..., freeze(x), ...).
+    // can cause oscillation between freeze(phi(...)) and phi(..., freeze(x),
+    // ...).
     if (isa<FreezeInst>(&I) && isGuaranteedNotToBeUndefOrPoison(InVal))
       return nullptr;

rahulana-quic · 2026-02-21T03:21:09Z

As mentioned in the issue, I was trying to reduce the testcase but I end up hitting the following from llvm-reduce after a few steps:

warning: input module no longer interesting after counting chunks

dtcxzyw · 2026-02-21T05:54:30Z

  ret i32 %y.fr
 }

+define i32 @trivial_incoming(i1 %cond, i32 %x) {


I cannot reproduce the issue: https://godbolt.org/z/o4h7dEoK9

Yes, the test added does not cause an infinite compile but it is supposed to be the test for the checks added in the patch. Please refer to the comment above wrt to a reproducible reduced test case.

dtcxzyw · 2026-02-21T06:00:48Z

warning: input module no longer interesting after counting chunks

Perhaps InstCombine will finally exit within a few seconds. It looks like a slow-compile issue like #164779 and #168849.

rahulana-quic · 2026-02-21T07:38:52Z

warning: input module no longer interesting after counting chunks

Perhaps InstCombine will finally exit within a few seconds. It looks like a slow-compile issue like #164779 and #168849.

I have confirmed this case to be an infinite compile by running opt -passes=instcombine on the module where we found the issue. From the bug I filed:

This issue was found while running spec2000/crafty with Ofast and thin-lto (in the function InitializeRandomHash from evaluate.c during the lto step).

Flags we use to build spec2000/crafty:
--target=arm-linux-gnueabi -mfloat-abi=softfp -mfpu=crypto-neon-fp-armv8 -mcpu=cortex-a53 -Ofast -flto=thin -Wno-error=incompatible-pointer-types -Wno-error=implicit-function-declaration -Wno-error=implicit-int -Wno-error=int-conversion -fdebug-default-version=4 -fcommon

nikic · 2026-02-21T08:43:05Z


+    // Don't push through an existing freeze. Otherwise we can repeatedly
+    // create redundant freeze instructions during instcombine iterations.
+    if (isa<FreezeInst>(V))


Can this actually happen? I'd expect this case to be folded by the preceding simplifyFreezeInst() call

nikic · 2026-02-21T08:50:14Z

As mentioned in the issue, I was trying to reduce the testcase but I end up hitting the following from llvm-reduce after a few steps:

warning: input module no longer interesting after counting chunks

This sounds like you reduced it to a test case that is right on the edge of your timeout value, so it will be interesting or not depending on noise.

I have confirmed this case to be an infinite compile by running opt -passes=instcombine on the module where we found the issue.

How long did you wait?

In any case, I'm personally inclined to revert #154336. This clearly has problematic interactions with other folds right now, and they are tricky to properly fix.

rahulana-quic · 2026-02-21T09:16:40Z

I have confirmed this case to be an infinite compile by running opt -passes=instcombine on the module where we found the issue.

How long did you wait?

I waited for more than 2 days and opt didn't terminate.

In any case, I'm personally inclined to revert #154336. This clearly has problematic interactions with other folds right now, and they are tricky to properly fix.

Makes sense, I saw some related PRs too where you have discussed this (#157678, #171435, etc.). Can we please get this reverted and possibly get the revert into the release branch as well?

apazos · 2026-02-22T19:41:44Z

Hi @nikic, the release is on Tuesday Feb 24th, it would be great if we can push a patch to revert [#154336] in the release 22.x branch (#154336) till a proper fix is merged.

This reverts commit f8f6965. This is causing infinte loops interacting with other transforms. See discussion on llvm#182647 .

…82769) This reverts commit f8f6965. This is causing infinte loops interacting with other transforms. See discussion on #182647 .

…llvm#182769) This reverts commit f8f6965. This is causing infinite loops interacting with other transforms. See discussion on llvm#182647 . (cherry picked from commit bd3b163)

rahulana-quic requested a review from nikic as a code owner February 21, 2026 02:50

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Feb 21, 2026

dtcxzyw reviewed Feb 21, 2026

View reviewed changes

nikic reviewed Feb 21, 2026

View reviewed changes

efriedma-quic mentioned this pull request Feb 22, 2026

Revert "[InstCombine] Allow freezing multiple operands (#154336)" #182769

Merged

efriedma-quic added a commit that referenced this pull request Feb 22, 2026

Revert "[InstCombine] Allow freezing multiple operands (#154336)" (#1…

bd3b163

…82769) This reverts commit f8f6965. This is causing infinte loops interacting with other transforms. See discussion on #182647 .

rahulana-quic closed this Feb 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[InstCombine] Fix non-convergence with freeze/phi folding#182647

[InstCombine] Fix non-convergence with freeze/phi folding#182647
rahulana-quic wants to merge 1 commit into
llvm:mainfrom
rahulana-quic:freeze_phi_fix

rahulana-quic commented Feb 21, 2026 •

edited

Loading

Uh oh!

llvmbot commented Feb 21, 2026

Uh oh!

github-actions Bot commented Feb 21, 2026

Uh oh!

github-actions Bot commented Feb 21, 2026

Uh oh!

rahulana-quic commented Feb 21, 2026

Uh oh!

dtcxzyw Feb 21, 2026

Uh oh!

rahulana-quic Feb 21, 2026

Uh oh!

dtcxzyw commented Feb 21, 2026

Uh oh!

rahulana-quic commented Feb 21, 2026

Uh oh!

nikic Feb 21, 2026

Uh oh!

nikic commented Feb 21, 2026

Uh oh!

rahulana-quic commented Feb 21, 2026

Uh oh!

apazos commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

rahulana-quic commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 21, 2026

Uh oh!

github-actions Bot commented Feb 21, 2026

Uh oh!

github-actions Bot commented Feb 21, 2026

Uh oh!

rahulana-quic commented Feb 21, 2026

Uh oh!

dtcxzyw Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

rahulana-quic Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

dtcxzyw commented Feb 21, 2026

Uh oh!

rahulana-quic commented Feb 21, 2026

Uh oh!

nikic Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

nikic commented Feb 21, 2026

Uh oh!

rahulana-quic commented Feb 21, 2026

Uh oh!

apazos commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rahulana-quic commented Feb 21, 2026 •

edited

Loading