From 3ece5e0ba55530147af2b393d216c379daf46908 Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Mon, 8 Sep 2025 10:40:48 +0000 Subject: [PATCH 1/7] [InstCombine] Push freeze through non-recurrence PHIs PR #157112 adds a test '@fold_phi_noundef_start_value' where we can't currently remove the freeze, even though it's possible. The two places in instcombine that deal with freezes are 'pushFreezeToPreventPoisonFromPropagating' and 'foldFreezeIntoRecurrence'. The former doesn't support PHIs at all and the latter is restricted to recurrences. It doesn't consider this test a recurrence as the BB of the frozen PHI node '%loop.latch' does not dominate either of the BBs ('%loop', '%if.else') of the incoming values. Therefore, I've updated 'pushFreezeToPreventPoisonFromPropagating' to support pushing the freeze to the incoming PHI values, as long as the BB of the frozen PHI *does not* dominate the BB of the incoming value(s). This fixes the test case added in #157112 and the other test changes look sensible, although perhaps I'm missing some edge cases (?). The 'match(U.get(), m_Undef())' check for undef PHI incoming value looks necessary to catch the case covered by '@two_undef' test in freeze-phi.ll. Without this check the undef becomes zero which doesnt seem right. --- .../InstCombine/InstructionCombining.cpp | 17 +++++++++++--- .../InstCombine/freeze-landingpad.ll | 5 ++-- .../test/Transforms/InstCombine/freeze-phi.ll | 7 +++--- llvm/test/Transforms/InstCombine/freeze.ll | 23 +++++++++---------- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index f0ddd5ca94c5a..bcd607d3e0245 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4976,10 +4976,18 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { // Op1.fr = Freeze(Op1) // ... = Inst(Op1.fr, NonPoisonOps...) - auto CanPushFreeze = [](Value *V) { - if (!isa(V) || isa(V)) + auto CanPushFreeze = [this](Value *V) { + if (!isa(V)) return false; + if (auto *PN = dyn_cast(V)) { + if (llvm::any_of(PN->incoming_values(), [this, &PN](Use &U) { + return DT.dominates(PN->getParent(), PN->getIncomingBlock(U)) || + match(U.get(), m_Undef()); + })) + 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 @@ -5004,7 +5012,10 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { return nullptr; auto *UserI = cast(U->getUser()); - Builder.SetInsertPoint(UserI); + if (auto *PN = dyn_cast(UserI)) + Builder.SetInsertPoint(PN->getIncomingBlock(*U)->getTerminator()); + else + Builder.SetInsertPoint(UserI); Value *Frozen = Builder.CreateFreeze(V, V->getName() + ".fr"); U->set(Frozen); continue; diff --git a/llvm/test/Transforms/InstCombine/freeze-landingpad.ll b/llvm/test/Transforms/InstCombine/freeze-landingpad.ll index 29167958c857f..259808c046490 100644 --- a/llvm/test/Transforms/InstCombine/freeze-landingpad.ll +++ b/llvm/test/Transforms/InstCombine/freeze-landingpad.ll @@ -14,14 +14,13 @@ define i32 @propagate_freeze_in_landingpad() personality ptr null { ; CHECK-NEXT: [[RES1:%.*]] = invoke i32 @foo() ; CHECK-NEXT: to label [[NORMAL_RETURN]] unwind label [[EXCEPTIONAL_RETURN]] ; CHECK: normal_return: -; CHECK-NEXT: [[INC]] = add nuw nsw i32 [[X]], 1 +; CHECK-NEXT: [[INC]] = add i32 [[X]], 1 ; CHECK-NEXT: br label [[INVOKE_BB1]] ; CHECK: exceptional_return: ; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[X]], [[INVOKE_BB1]] ], [ 0, [[INVOKE_BB2]] ] ; CHECK-NEXT: [[LANDING_PAD:%.*]] = landingpad { ptr, i32 } ; CHECK-NEXT: cleanup -; CHECK-NEXT: [[FR:%.*]] = freeze i32 [[PHI]] -; CHECK-NEXT: [[RES:%.*]] = shl i32 [[FR]], 1 +; CHECK-NEXT: [[RES:%.*]] = shl i32 [[PHI]], 1 ; CHECK-NEXT: ret i32 [[RES]] ; entry: diff --git a/llvm/test/Transforms/InstCombine/freeze-phi.ll b/llvm/test/Transforms/InstCombine/freeze-phi.ll index cdc9a5efe5933..098e6c5f6cdbb 100644 --- a/llvm/test/Transforms/InstCombine/freeze-phi.ll +++ b/llvm/test/Transforms/InstCombine/freeze-phi.ll @@ -94,13 +94,14 @@ define i32 @two(i1 %cond, i32 %x, i32 %x2) { ; CHECK-LABEL: @two( ; CHECK-NEXT: br i1 [[COND:%.*]], label [[A:%.*]], label [[B:%.*]] ; CHECK: A: +; CHECK-NEXT: [[X_FR:%.*]] = freeze i32 [[X:%.*]] ; CHECK-NEXT: br label [[C:%.*]] ; CHECK: B: +; CHECK-NEXT: [[X2_FR:%.*]] = freeze i32 [[X2:%.*]] ; CHECK-NEXT: br label [[C]] ; CHECK: C: -; CHECK-NEXT: [[Y:%.*]] = phi i32 [ [[X:%.*]], [[A]] ], [ [[X2:%.*]], [[B]] ] -; CHECK-NEXT: [[Y_FR:%.*]] = freeze i32 [[Y]] -; CHECK-NEXT: ret i32 [[Y_FR]] +; CHECK-NEXT: [[Y:%.*]] = phi i32 [ [[X_FR]], [[A]] ], [ [[X2_FR]], [[B]] ] +; CHECK-NEXT: ret i32 [[Y]] ; br i1 %cond, label %A, label %B A: diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll index af5cb0c75537b..91e89b3b5a332 100644 --- a/llvm/test/Transforms/InstCombine/freeze.ll +++ b/llvm/test/Transforms/InstCombine/freeze.ll @@ -269,15 +269,15 @@ define void @freeze_dominated_uses_catchswitch(i1 %c, i32 %x) personality ptr @_ ; CHECK-NEXT: invoke void @use_i32(i32 0) ; CHECK-NEXT: to label %[[CLEANUP:.*]] unwind label %[[CATCH_DISPATCH:.*]] ; CHECK: [[IF_ELSE]]: +; CHECK-NEXT: [[X_FR:%.*]] = freeze i32 [[X]] ; CHECK-NEXT: invoke void @use_i32(i32 1) ; 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: [[PHI:%.*]] = phi i32 [ 0, %[[IF_THEN]] ], [ [[X_FR]], %[[IF_ELSE]] ] ; 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: call void @use_i32(i32 [[PHI]]) [ "funclet"(token [[CP]]) ] ; CHECK-NEXT: unreachable ; CHECK: [[CATCH2:.*:]] ; CHECK-NEXT: [[CP2:%.*]] = catchpad within [[CS]] [ptr null, i32 64, ptr null] @@ -384,15 +384,16 @@ define i32 @freeze_phi_followed_by_phi(i1 %c, i32 %y, i32 %z) { ; CHECK-LABEL: define i32 @freeze_phi_followed_by_phi( ; CHECK-SAME: i1 [[C:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) { ; CHECK-NEXT: [[ENTRY:.*]]: +; CHECK-NEXT: [[Z_FR:%.*]] = freeze i32 [[Z]] +; CHECK-NEXT: [[Y_FR:%.*]] = freeze i32 [[Y]] ; CHECK-NEXT: br i1 [[C]], label %[[IF:.*]], label %[[JOIN:.*]] ; CHECK: [[IF]]: ; CHECK-NEXT: br label %[[JOIN]] ; CHECK: [[JOIN]]: -; CHECK-NEXT: [[X:%.*]] = phi i32 [ [[Y]], %[[IF]] ], [ [[Z]], %[[ENTRY]] ] -; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[Z]], %[[IF]] ], [ [[Y]], %[[ENTRY]] ] -; CHECK-NEXT: [[FR:%.*]] = freeze i32 [[X]] -; CHECK-NEXT: call void @use_i32(i32 [[FR]]) -; CHECK-NEXT: call void @use_i32(i32 [[FR]]) +; CHECK-NEXT: [[X:%.*]] = phi i32 [ [[Y_FR]], %[[IF]] ], [ [[Z_FR]], %[[ENTRY]] ] +; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[Z_FR]], %[[IF]] ], [ [[Y_FR]], %[[ENTRY]] ] +; CHECK-NEXT: call void @use_i32(i32 [[X]]) +; CHECK-NEXT: call void @use_i32(i32 [[X]]) ; CHECK-NEXT: ret i32 [[PHI]] ; entry: @@ -1141,8 +1142,7 @@ exit: } ; We can remove this freeze as the incoming values to the PHI have the same -; well-defined start value and the GEP can't produce poison, but this is -; currently unsupported. +; well-defined start value and the GEP can't produce poison. define void @fold_phi_noundef_start_value(ptr noundef %init, i1 %cond.0, i1 %cond.1, i1 %cond.2) { ; CHECK-LABEL: define void @fold_phi_noundef_start_value( ; CHECK-SAME: ptr noundef [[INIT:%.*]], i1 [[COND_0:%.*]], i1 [[COND_1:%.*]], i1 [[COND_2:%.*]]) { @@ -1156,8 +1156,7 @@ define void @fold_phi_noundef_start_value(ptr noundef %init, i1 %cond.0, i1 %con ; CHECK-NEXT: br label %[[LOOP_LATCH]] ; 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_2_FR_INT:%.*]] = ptrtoint ptr [[IV_2]] 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]] From 83986a4889fb4b9f42860ab724e6f1af10d5d7c2 Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Wed, 10 Sep 2025 06:53:48 +0000 Subject: [PATCH 2/7] address comments - remove unused %cond.2 from tests - bail on backedge from BB of incoming value of phi to BB of freeze - bail if incoming value of phi is from invoke and add a test - bail if phi has multiple values from same predecessor and test --- .../InstCombine/InstructionCombining.cpp | 15 ++- llvm/test/Transforms/InstCombine/freeze.ll | 121 +++++++++++++++++- 2 files changed, 127 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index bcd607d3e0245..efef4e0d4e3a2 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4981,11 +4981,16 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { return false; if (auto *PN = dyn_cast(V)) { - if (llvm::any_of(PN->incoming_values(), [this, &PN](Use &U) { - return DT.dominates(PN->getParent(), PN->getIncomingBlock(U)) || - match(U.get(), m_Undef()); - })) - return false; + BasicBlock *BB = PN->getParent(); + SmallPtrSet VisitedBBs; + for (Use &U : PN->incoming_values()) { + BasicBlock *InBB = PN->getIncomingBlock(U); + if (DT.dominates(BB, InBB) || isBackEdge(InBB, BB) || + isa(U) || VisitedBBs.contains(InBB) || + match(U.get(), m_Undef())) + return false; + VisitedBBs.insert(InBB); + } } // We can't push the freeze through an instruction which can itself create diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll index 91e89b3b5a332..19beb01e0b99f 100644 --- a/llvm/test/Transforms/InstCombine/freeze.ll +++ b/llvm/test/Transforms/InstCombine/freeze.ll @@ -1143,9 +1143,9 @@ exit: ; We can remove this freeze as the incoming values to the PHI have the same ; well-defined start value and the GEP can't produce poison. -define void @fold_phi_noundef_start_value(ptr noundef %init, i1 %cond.0, i1 %cond.1, i1 %cond.2) { +define void @fold_phi_noundef_start_value(ptr noundef %init, i1 %cond.0, i1 %cond.1) { ; CHECK-LABEL: define void @fold_phi_noundef_start_value( -; CHECK-SAME: ptr noundef [[INIT:%.*]], i1 [[COND_0:%.*]], i1 [[COND_1:%.*]], i1 [[COND_2:%.*]]) { +; CHECK-SAME: ptr noundef [[INIT:%.*]], i1 [[COND_0:%.*]], i1 [[COND_1:%.*]]) { ; CHECK-NEXT: [[ENTRY:.*]]: ; CHECK-NEXT: br label %[[LOOP:.*]] ; CHECK: [[LOOP]]: @@ -1160,7 +1160,7 @@ define void @fold_phi_noundef_start_value(ptr noundef %init, i1 %cond.0, i1 %con ; 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_2]], label %[[EXIT:.*]], label %[[LOOP]] +; CHECK-NEXT: br i1 [[COND_1]], label %[[EXIT:.*]], label %[[LOOP]] ; CHECK: [[EXIT]]: ; CHECK-NEXT: ret void ; @@ -1182,7 +1182,120 @@ loop.latch: %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.2, label %exit, label %loop + br i1 %cond.1, label %exit, label %loop + +exit: + ret void +} + +declare ptr @get_ptr() + +; When the phi isn't a simple recurrence and has multiple inputs from the same +; predecessor, we need to be careful to avoid iterator invalidation. The phi +; must have identical values for the predecessor and at no point should the +; freeze be pushed to a single one of the uses, e.g. +; +; %iv.2 = phi ptr [ %iv.0, %loop ], [ %iv.1.fr, %if.else ], [ %iv.1, %if.else ] +; +; We simply don't support this case, although it could be handled if there's a +; use case. +define void @fold_phi_non_simple_recurrence_multiple_forward_edges(ptr noundef %init, i1 %cond.0, i1 %cond.1) { +; CHECK-LABEL: define void @fold_phi_non_simple_recurrence_multiple_forward_edges( +; CHECK-SAME: ptr noundef [[INIT:%.*]], i1 [[COND_0:%.*]], i1 [[COND_1:%.*]]) { +; 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:%.*]] = call ptr @get_ptr() +; CHECK-NEXT: br i1 false, label %[[LOOP_LATCH]], label %[[LOOP_LATCH]] +; CHECK: [[LOOP_LATCH]]: +; CHECK-NEXT: [[IV_2:%.*]] = phi ptr [ [[IV_0]], %[[LOOP]] ], [ [[IV_1]], %[[IF_ELSE]] ], [ [[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: [[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 = call ptr @get_ptr() + br i1 %cond.0, label %loop.latch, label %loop.latch + +loop.latch: + %iv.2 = phi ptr [ %iv.0, %loop ], [ %iv.1, %if.else ], [ %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 + +exit: + ret void +} + +; 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 From 73e6bc6f903da75cd5671a286ce15645dfedd31e Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Wed, 10 Sep 2025 14:24:36 +0000 Subject: [PATCH 3/7] replace isa with generic check for terminator --- .../Transforms/InstCombine/InstructionCombining.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index efef4e0d4e3a2..0998a8fdf3a9a 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4985,9 +4985,15 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { SmallPtrSet VisitedBBs; for (Use &U : PN->incoming_values()) { BasicBlock *InBB = PN->getIncomingBlock(U); + // We can't move freeze if the start value is the result of a + // terminator (e.g. an invoke). + if (auto *OpI = dyn_cast(U)) { + if (OpI->isTerminator()) + return false; + } + if (DT.dominates(BB, InBB) || isBackEdge(InBB, BB) || - isa(U) || VisitedBBs.contains(InBB) || - match(U.get(), m_Undef())) + VisitedBBs.contains(InBB) || match(U.get(), m_Undef())) return false; VisitedBBs.insert(InBB); } From c0d7cde73792121a8accfbad6aeb1e6901a6dc08 Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Mon, 22 Sep 2025 09:11:34 +0000 Subject: [PATCH 4/7] address comments --- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 0998a8fdf3a9a..6004feb1d8851 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4992,8 +4992,12 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { return false; } - if (DT.dominates(BB, InBB) || isBackEdge(InBB, BB) || - VisitedBBs.contains(InBB) || match(U.get(), m_Undef())) + // If there's multiple incoming edges from the same predecessor we must + // ensure the freeze isn't pushed to a single one of the uses, + // invalidating the iterator. We simply don't support this case, but it + // could be handled if there's a use case. + if (isBackEdge(InBB, BB) || !VisitedBBs.insert(InBB).second || + match(U.get(), m_Undef())) return false; VisitedBBs.insert(InBB); } From df6755da3c6aca35657dd45b46f0d6a01f93504c Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Tue, 23 Sep 2025 14:04:40 +0000 Subject: [PATCH 5/7] address comments --- .../Transforms/InstCombine/InstructionCombining.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 6004feb1d8851..85a05e399668d 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4983,11 +4983,13 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { if (auto *PN = dyn_cast(V)) { BasicBlock *BB = PN->getParent(); SmallPtrSet VisitedBBs; - for (Use &U : PN->incoming_values()) { - BasicBlock *InBB = PN->getIncomingBlock(U); + for (unsigned I = 0; I < PN->getNumIncomingValues(); ++I) { + Value *InV = PN->getIncomingValue(I); + BasicBlock *InBB = PN->getIncomingBlock(I); + // We can't move freeze if the start value is the result of a // terminator (e.g. an invoke). - if (auto *OpI = dyn_cast(U)) { + if (auto *OpI = dyn_cast(InV)) { if (OpI->isTerminator()) return false; } @@ -4997,7 +4999,7 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { // invalidating the iterator. We simply don't support this case, but it // could be handled if there's a use case. if (isBackEdge(InBB, BB) || !VisitedBBs.insert(InBB).second || - match(U.get(), m_Undef())) + match(InV, m_Undef())) return false; VisitedBBs.insert(InBB); } From 8b9e1ec058347eabac7880d297eacd0a44a9f4c6 Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Tue, 23 Sep 2025 14:46:06 +0000 Subject: [PATCH 6/7] Revert "address comments" This reverts commit 790e7ede98c32b4a8c2f0fa69a2817edfc63e475. --- .../Transforms/InstCombine/InstructionCombining.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 85a05e399668d..6004feb1d8851 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4983,13 +4983,11 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { if (auto *PN = dyn_cast(V)) { BasicBlock *BB = PN->getParent(); SmallPtrSet VisitedBBs; - for (unsigned I = 0; I < PN->getNumIncomingValues(); ++I) { - Value *InV = PN->getIncomingValue(I); - BasicBlock *InBB = PN->getIncomingBlock(I); - + for (Use &U : PN->incoming_values()) { + BasicBlock *InBB = PN->getIncomingBlock(U); // We can't move freeze if the start value is the result of a // terminator (e.g. an invoke). - if (auto *OpI = dyn_cast(InV)) { + if (auto *OpI = dyn_cast(U)) { if (OpI->isTerminator()) return false; } @@ -4999,7 +4997,7 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { // invalidating the iterator. We simply don't support this case, but it // could be handled if there's a use case. if (isBackEdge(InBB, BB) || !VisitedBBs.insert(InBB).second || - match(InV, m_Undef())) + match(U.get(), m_Undef())) return false; VisitedBBs.insert(InBB); } From 05d8a513fafc327bdf93935a5d23aafb6be8c7f1 Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Tue, 30 Sep 2025 12:05:54 +0000 Subject: [PATCH 7/7] fix compile-time issue in spec2017 527.cam4_r --- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 6004feb1d8851..b830b1694f2da 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -5026,6 +5026,9 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { if (U == OrigUse) return nullptr; + if (isa(V)) + continue; + auto *UserI = cast(U->getUser()); if (auto *PN = dyn_cast(UserI)) Builder.SetInsertPoint(PN->getIncomingBlock(*U)->getTerminator());