[llvm] [InstCombine] Merge foldFreezeIntoRecurrence into pushFreezeToPreventPoisonFromPropagating (PR #171435)
Cullen Rhodes via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 9 09:33:36 PST 2025
https://github.com/c-rhodes updated https://github.com/llvm/llvm-project/pull/171435
>From dd43c790be7b49fbff70bda21b2f002ecd682ef3 Mon Sep 17 00:00:00 2001
From: Cullen Rhodes <cullen.rhodes at arm.com>
Date: Tue, 9 Dec 2025 12:39:07 +0000
Subject: [PATCH 1/3] Regenerate llvm/test/Transforms/InstCombine/freeze.ll
---
llvm/test/Transforms/InstCombine/freeze.ll | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll
index ac7d65c2a3c6a..0d75a79d59a94 100644
--- a/llvm/test/Transforms/InstCombine/freeze.ll
+++ b/llvm/test/Transforms/InstCombine/freeze.ll
@@ -273,13 +273,13 @@ define void @freeze_dominated_uses_catchswitch(i1 %c, i32 %x) personality ptr @_
; CHECK-NEXT: to label %[[CLEANUP]] unwind label %[[CATCH_DISPATCH]]
; CHECK: [[CATCH_DISPATCH]]:
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ 0, %[[IF_THEN]] ], [ [[X]], %[[IF_ELSE]] ]
-; CHECK-NEXT: [[CS:%.*]] = catchswitch within none [label %[[CATCH:.*]], label %catch2] unwind to caller
+; CHECK-NEXT: [[CS:%.*]] = catchswitch within none [label %[[CATCH:.*]], label %[[CATCH2:.*]]] unwind to caller
; CHECK: [[CATCH]]:
; CHECK-NEXT: [[CP:%.*]] = catchpad within [[CS]] [ptr null, i32 64, ptr null]
; CHECK-NEXT: [[PHI_FREEZE:%.*]] = freeze i32 [[PHI]]
; CHECK-NEXT: call void @use_i32(i32 [[PHI_FREEZE]]) [ "funclet"(token [[CP]]) ]
; CHECK-NEXT: unreachable
-; CHECK: [[CATCH2:.*:]]
+; CHECK: [[CATCH2]]:
; CHECK-NEXT: [[CP2:%.*]] = catchpad within [[CS]] [ptr null, i32 64, ptr null]
; CHECK-NEXT: call void @use_i32(i32 [[PHI]]) [ "funclet"(token [[CP2]]) ]
; CHECK-NEXT: unreachable
>From fa2fe17555c237618c95fadd8c09aa847fee1c34 Mon Sep 17 00:00:00 2001
From: Cullen Rhodes <cullen.rhodes at arm.com>
Date: Tue, 9 Dec 2025 12:05:18 +0000
Subject: [PATCH 2/3] [InstCombine] Merge foldFreezeIntoRecurrence into
pushFreezeToPreventPoisonFromPropagating
---
.../InstCombine/InstructionCombining.cpp | 118 +++++++-----------
llvm/test/Transforms/InstCombine/freeze.ll | 58 +++++++++
2 files changed, 102 insertions(+), 74 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index c6de57cb34c69..4e992fba75962 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -5027,10 +5027,44 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
// Op1.fr = Freeze(Op1)
// ... = Inst(Op1.fr, NonPoisonOps...)
- auto CanPushFreeze = [](Value *V) {
- if (!isa<Instruction>(V) || isa<PHINode>(V))
+ auto CanPushFreeze = [this](Value *V) {
+ if (!isa<Instruction>(V))
return false;
+ if (auto *PN = dyn_cast<PHINode>(V)) {
+ // Detect whether this is a recurrence with a start value and some number
+ // of backedge values. We'll check whether we can push the freeze through
+ // the backedge values (possibly dropping poison flags along the way)
+ // until we reach the phi again. In that case, we can move the freeze to
+ // the start value.
+ Use *StartU = nullptr;
+ bool HasBackedge = false;
+ for (Use &U : PN->incoming_values()) {
+ if (DT.dominates(PN->getParent(), PN->getIncomingBlock(U))) {
+ HasBackedge = true;
+ continue;
+ }
+
+ // Don't bother handling multiple start values.
+ if (StartU)
+ return false;
+ StartU = &U;
+ }
+
+ if (!StartU || !HasBackedge)
+ return false; // Not a recurrence.
+
+ Value *StartV = StartU->get();
+ BasicBlock *StartBB = PN->getIncomingBlock(*StartU);
+ bool StartNeedsFreeze = !isGuaranteedNotToBeUndefOrPoison(StartV);
+ // We can't insert freeze if the start value is the result of the
+ // terminator (e.g. an invoke).
+ if (StartNeedsFreeze && StartBB->getTerminator() == StartV)
+ return false;
+
+ return true;
+ }
+
// 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
@@ -5043,7 +5077,7 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
// we directly push the freeze all the way to the leaves. However, we leave
// deduplication of freezes on the same value for freezeOtherUses().
Use *OrigUse = &OrigFI.getOperandUse(0);
- SmallPtrSet<Instruction *, 8> Visited;
+ SmallPtrSet<Instruction *, 32> Visited;
SmallVector<Use *, 8> Worklist;
Worklist.push_back(OrigUse);
while (!Worklist.empty()) {
@@ -5055,7 +5089,10 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
return nullptr;
auto *UserI = cast<Instruction>(U->getUser());
- Builder.SetInsertPoint(UserI);
+ if (auto *PN = dyn_cast<PHINode>(UserI))
+ Builder.SetInsertPoint(PN->getIncomingBlock(*U)->getTerminator());
+ else
+ Builder.SetInsertPoint(UserI);
Value *Frozen = Builder.CreateFreeze(V, V->getName() + ".fr");
U->set(Frozen);
continue;
@@ -5065,6 +5102,9 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
if (!Visited.insert(I).second)
continue;
+ if (Visited.size() > 32)
+ return nullptr; // Limit the total number of values we inspect.
+
// reverse() to emit freezes in a more natural order.
for (Use &Op : reverse(I->operands())) {
Value *OpV = Op.get();
@@ -5080,74 +5120,6 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
return OrigUse->get();
}
-Instruction *InstCombinerImpl::foldFreezeIntoRecurrence(FreezeInst &FI,
- PHINode *PN) {
- // Detect whether this is a recurrence with a start value and some number of
- // backedge values. We'll check whether we can push the freeze through the
- // backedge values (possibly dropping poison flags along the way) until we
- // reach the phi again. In that case, we can move the freeze to the start
- // value.
- Use *StartU = nullptr;
- SmallVector<Value *> Worklist;
- for (Use &U : PN->incoming_values()) {
- if (DT.dominates(PN->getParent(), PN->getIncomingBlock(U))) {
- // Add backedge value to worklist.
- Worklist.push_back(U.get());
- continue;
- }
-
- // Don't bother handling multiple start values.
- if (StartU)
- return nullptr;
- StartU = &U;
- }
-
- if (!StartU || Worklist.empty())
- return nullptr; // Not a recurrence.
-
- Value *StartV = StartU->get();
- BasicBlock *StartBB = PN->getIncomingBlock(*StartU);
- bool StartNeedsFreeze = !isGuaranteedNotToBeUndefOrPoison(StartV);
- // We can't insert freeze if the start value is the result of the
- // terminator (e.g. an invoke).
- if (StartNeedsFreeze && StartBB->getTerminator() == StartV)
- return nullptr;
-
- SmallPtrSet<Value *, 32> Visited;
- SmallVector<Instruction *> DropFlags;
- while (!Worklist.empty()) {
- Value *V = Worklist.pop_back_val();
- if (!Visited.insert(V).second)
- continue;
-
- if (Visited.size() > 32)
- return nullptr; // Limit the total number of values we inspect.
-
- // Assume that PN is non-poison, because it will be after the transform.
- if (V == PN || isGuaranteedNotToBeUndefOrPoison(V))
- continue;
-
- Instruction *I = dyn_cast<Instruction>(V);
- if (!I || canCreateUndefOrPoison(cast<Operator>(I),
- /*ConsiderFlagsAndMetadata*/ false))
- return nullptr;
-
- DropFlags.push_back(I);
- append_range(Worklist, I->operands());
- }
-
- for (Instruction *I : DropFlags)
- I->dropPoisonGeneratingAnnotations();
-
- if (StartNeedsFreeze) {
- Builder.SetInsertPoint(StartBB->getTerminator());
- Value *FrozenStartV = Builder.CreateFreeze(StartV,
- StartV->getName() + ".fr");
- replaceUse(*StartU, FrozenStartV);
- }
- return replaceInstUsesWith(FI, PN);
-}
-
bool InstCombinerImpl::freezeOtherUses(FreezeInst &FI) {
Value *Op = FI.getOperand(0);
@@ -5209,8 +5181,6 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) {
if (auto *PN = dyn_cast<PHINode>(Op0)) {
if (Instruction *NV = foldOpIntoPhi(I, PN))
return NV;
- if (Instruction *NV = foldFreezeIntoRecurrence(I, PN))
- return NV;
}
if (Value *NI = pushFreezeToPreventPoisonFromPropagating(I))
diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll
index 0d75a79d59a94..c15ef8e52a0bd 100644
--- a/llvm/test/Transforms/InstCombine/freeze.ll
+++ b/llvm/test/Transforms/InstCombine/freeze.ll
@@ -1189,6 +1189,64 @@ exit:
ret void
}
+declare ptr @get_ptr()
+
+; When the phi input comes from an invoke, we need to be careful the freeze
+; isn't pushed after the invoke.
+define void @fold_phi_noundef_start_value_with_invoke(ptr noundef %init, i1 %cond.0, i1 %cond.1) personality ptr undef {
+; CHECK-LABEL: define void @fold_phi_noundef_start_value_with_invoke(
+; CHECK-SAME: ptr noundef [[INIT:%.*]], i1 [[COND_0:%.*]], i1 [[COND_1:%.*]]) personality ptr undef {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: br label %[[LOOP:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[IV_0:%.*]] = phi ptr [ [[INIT]], %[[ENTRY]] ], [ [[IV_0_NEXT:%.*]], %[[LOOP_LATCH:.*]] ]
+; CHECK-NEXT: br i1 [[COND_0]], label %[[LOOP_LATCH]], label %[[IF_ELSE:.*]]
+; CHECK: [[IF_ELSE]]:
+; CHECK-NEXT: [[IV_1:%.*]] = invoke ptr @get_ptr()
+; CHECK-NEXT: to label %[[LOOP_LATCH]] unwind label %[[UNWIND:.*]]
+; CHECK: [[LOOP_LATCH]]:
+; CHECK-NEXT: [[IV_2:%.*]] = phi ptr [ [[IV_0]], %[[LOOP]] ], [ [[IV_1]], %[[IF_ELSE]] ]
+; CHECK-NEXT: [[IV_2_FR:%.*]] = freeze ptr [[IV_2]]
+; CHECK-NEXT: [[IV_2_FR_INT:%.*]] = ptrtoint ptr [[IV_2_FR]] to i64
+; CHECK-NEXT: [[IV_0_INT:%.*]] = ptrtoint ptr [[IV_0]] to i64
+; CHECK-NEXT: [[IDX:%.*]] = sub i64 [[IV_0_INT]], [[IV_2_FR_INT]]
+; CHECK-NEXT: [[IV_0_NEXT]] = getelementptr i8, ptr [[IV_0]], i64 [[IDX]]
+; CHECK-NEXT: br i1 [[COND_1]], label %[[EXIT:.*]], label %[[LOOP]]
+; CHECK: [[UNWIND]]:
+; CHECK-NEXT: [[TMP0:%.*]] = landingpad i8
+; CHECK-NEXT: cleanup
+; CHECK-NEXT: unreachable
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: ret void
+;
+entry:
+ br label %loop
+
+loop:
+ %iv.0 = phi ptr [ %init, %entry ], [ %iv.0.next, %loop.latch ]
+ br i1 %cond.0, label %loop.latch, label %if.else
+
+if.else:
+ %iv.1 = invoke ptr @get_ptr()
+ to label %loop.latch unwind label %unwind
+
+loop.latch:
+ %iv.2 = phi ptr [ %iv.0, %loop ], [ %iv.1, %if.else ]
+ %iv.2.fr = freeze ptr %iv.2
+ %iv.2.fr.int = ptrtoint ptr %iv.2.fr to i64
+ %iv.0.int = ptrtoint ptr %iv.0 to i64
+ %idx = sub i64 %iv.0.int, %iv.2.fr.int
+ %iv.0.next = getelementptr i8, ptr %iv.0, i64 %idx
+ br i1 %cond.1, label %exit, label %loop
+
+unwind:
+ landingpad i8 cleanup
+ unreachable
+
+exit:
+ ret void
+}
+
define void @fold_phi_invoke_start_value(i32 %n) personality ptr undef {
; CHECK-LABEL: define void @fold_phi_invoke_start_value(
; CHECK-SAME: i32 [[N:%.*]]) personality ptr undef {
>From 5493e98f946f8b65ea1a56f75c8bb788b5faf298 Mon Sep 17 00:00:00 2001
From: Cullen Rhodes <cullen.rhodes at arm.com>
Date: Tue, 9 Dec 2025 17:13:06 +0000
Subject: [PATCH 3/3] Bail if CanPushFreeze returns false for PHI
Looking at this code again the Use/User distinction is a bit confusing,
but I'm pretty sure if we bail when CanPushFreeze returns false for the
PHI it should bring the behaviour inline with what was happening when
this code was part of foldFreezeIntoRecurrence.
Change was causing compile-time issues:
https://github.com/dtcxzyw/llvm-opt-benchmark/pull/3129
The smallest being:
https://github.com/dtcxzyw/llvm-opt-benchmark/blob/main/bench/ffmpeg/original/avc.ll
which I reduced to: https://godbolt.org/z/6Gx58GfK1
---
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 4e992fba75962..5139dd82417b6 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -5084,8 +5084,9 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
auto *U = Worklist.pop_back_val();
Value *V = U->get();
if (!CanPushFreeze(V)) {
- // If we can't push through the original instruction, abort the transform.
- if (U == OrigUse)
+ // If we can't push through the original instruction or a PHI, abort the
+ // transform.
+ if (U == OrigUse || isa<PHINode>(V))
return nullptr;
auto *UserI = cast<Instruction>(U->getUser());
More information about the llvm-commits
mailing list