From d1a6a71067128514f40465b54eee1f0710b2e555 Mon Sep 17 00:00:00 2001 From: Mikael Nilsson Date: Thu, 25 Sep 2025 15:35:47 +0200 Subject: [PATCH 1/5] [InstCombine] Opt phi(freeze(undef), C) -> phi(C, C) Change-Id: I29fd2663a5c0e4e3a6d98a2c9536f515a12947a5 --- .../InstCombine/InstructionCombining.cpp | 45 +++- .../Transforms/InstCombine/in-freeze-phi.ll | 238 ++++++++++++++++++ 2 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 llvm/test/Transforms/InstCombine/in-freeze-phi.ll diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 9ddcef0396e39..5cd6aeb301173 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4821,8 +4821,51 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { // TODO: This could use getBinopAbsorber() / getBinopIdentity() to avoid // duplicating logic for binops at least. auto getUndefReplacement = [&](Type *Ty) { - Value *BestValue = nullptr; + auto pickCommonConstantFromPHI = [](PHINode &PN) -> Value * { + // phi(freeze(undef), C, C). Choose C for freeze so the PHI can be + // removed. + Constant *BestValue = nullptr; + Constant *C = nullptr; + for (Value *V : PN.incoming_values()) { + if (match(V, m_Freeze(m_Undef()))) + continue; + + if (!isa(V)) + return nullptr; + + C = cast(V); + + if (BestValue && BestValue != C) + return nullptr; + + BestValue = C; + } + return BestValue; + }; + Value *NullValue = Constant::getNullValue(Ty); + + bool OnlyPHIUsers = + all_of(I.users(), [](const User *U) { return isa(U); }); + if (OnlyPHIUsers) { + Value *BestValue = nullptr; + for (auto *U : I.users()) { + Value *V = pickCommonConstantFromPHI(*cast(U)); + if (!V) + continue; + + if (!BestValue) { + BestValue = V; + } else if (BestValue && BestValue == NullValue) { + BestValue = NullValue; + } + } + + if (BestValue) + return BestValue; + } + + Value *BestValue = nullptr; for (const auto *U : I.users()) { Value *V = NullValue; if (match(U, m_Or(m_Value(), m_Value()))) diff --git a/llvm/test/Transforms/InstCombine/in-freeze-phi.ll b/llvm/test/Transforms/InstCombine/in-freeze-phi.ll new file mode 100644 index 0000000000000..261aafa179cb7 --- /dev/null +++ b/llvm/test/Transforms/InstCombine/in-freeze-phi.ll @@ -0,0 +1,238 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 +; RUN: opt -passes=instcombine -S < %s | FileCheck %s + +define i32 @phi_freeze_same_consts(i1 %c0, i1 %c1) { +; CHECK-LABEL: define i32 @phi_freeze_same_consts( +; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C0]], label %[[BB_FREEZE:.*]], label %[[BB_OTHER:.*]] +; CHECK: [[BB_FREEZE]]: +; CHECK-NEXT: br label %[[FINAL:.*]] +; CHECK: [[BB_OTHER]]: +; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] +; CHECK: [[CA]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[CB]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[FINAL]]: +; CHECK-NEXT: ret i32 42 +; +entry: + br i1 %c0, label %bb_freeze, label %bb_other + +bb_freeze: + %f = freeze i32 undef + br label %final + +bb_other: + br i1 %c1, label %cA, label %cB +cA: + br label %final +cB: + br label %final + +final: + %phi = phi i32 [ %f, %bb_freeze ], [ 42, %cA ], [ 42, %cB ] + ret i32 %phi +} + +define i32 @phi_freeze_mixed_consts(i1 %c0, i1 %c1) { +; CHECK-LABEL: define i32 @phi_freeze_mixed_consts( +; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C0]], label %[[BB_FREEZE:.*]], label %[[BB_OTHER:.*]] +; CHECK: [[BB_FREEZE]]: +; CHECK-NEXT: br label %[[FINAL:.*]] +; CHECK: [[BB_OTHER]]: +; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] +; CHECK: [[CA]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[CB]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[FINAL]]: +; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ 0, %[[BB_FREEZE]] ], [ 42, %[[CA]] ], [ 7, %[[CB]] ] +; CHECK-NEXT: ret i32 [[PHI]] +; +entry: + br i1 %c0, label %bb_freeze, label %bb_other + +bb_freeze: + %f = freeze i32 undef + br label %final + +bb_other: + br i1 %c1, label %cA, label %cB +cA: + br label %final +cB: + br label %final + +final: + %phi = phi i32 [ %f, %bb_freeze ], [ 42, %cA ], [ 7, %cB ] + ret i32 %phi +} + +define i32 @phi_freeze_with_nonconst_incoming(i32 %x, i1 %c0, i1 %c1) { +; CHECK-LABEL: define i32 @phi_freeze_with_nonconst_incoming( +; CHECK-SAME: i32 [[X:%.*]], i1 [[C0:%.*]], i1 [[C1:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C0]], label %[[BB_FREEZE:.*]], label %[[BB_OTHER:.*]] +; CHECK: [[BB_FREEZE]]: +; CHECK-NEXT: br label %[[FINAL:.*]] +; CHECK: [[BB_OTHER]]: +; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] +; CHECK: [[CA]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[CB]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[FINAL]]: +; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ 0, %[[BB_FREEZE]] ], [ [[X]], %[[CA]] ], [ 13, %[[CB]] ] +; CHECK-NEXT: ret i32 [[PHI]] +; +entry: + br i1 %c0, label %bb_freeze, label %bb_other + +bb_freeze: + %f = freeze i32 undef + br label %final + +bb_other: + br i1 %c1, label %cA, label %cB +cA: + br label %final +cB: + br label %final + +final: + %phi = phi i32 [ %f, %bb_freeze ], [ %x, %cA ], [ 13, %cB ] + ret i32 %phi +} + +define <4 x i8> @phi_freeze_vector(i1 %c0, i1 %c1) { +; CHECK-LABEL: define <4 x i8> @phi_freeze_vector( +; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C0]], label %[[BB_FREEZE:.*]], label %[[BB_OTHER:.*]] +; CHECK: [[BB_FREEZE]]: +; CHECK-NEXT: br label %[[FINAL:.*]] +; CHECK: [[BB_OTHER]]: +; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] +; CHECK: [[CA]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[CB]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[FINAL]]: +; CHECK-NEXT: ret <4 x i8> splat (i8 9) +; +entry: + br i1 %c0, label %bb_freeze, label %bb_other + +bb_freeze: + %f = freeze <4 x i8> undef + br label %final + +bb_other: + br i1 %c1, label %cA, label %cB + +cA: + br label %final + +cB: + br label %final + +final: + %phi = phi <4 x i8> [ %f, %bb_freeze ], + [, %cA ], + [, %cB ] + ret <4 x i8> %phi +} + +define i32 @multi_use_one_folds_one_not(i1 %c0, i1 %c1) { +; CHECK-LABEL: define i32 @multi_use_one_folds_one_not( +; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C0]], label %[[BB_FREEZE:.*]], label %[[BB_OTHER:.*]] +; CHECK: [[BB_FREEZE]]: +; CHECK-NEXT: br label %[[MID:.*]] +; CHECK: [[BB_OTHER]]: +; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] +; CHECK: [[CA]]: +; CHECK-NEXT: br label %[[MID]] +; CHECK: [[CB]]: +; CHECK-NEXT: br label %[[MID]] +; CHECK: [[MID]]: +; CHECK-NEXT: [[A:%.*]] = phi i32 [ 2, %[[BB_FREEZE]] ], [ 4, %[[CA]] ], [ 3, %[[CB]] ] +; CHECK-NEXT: ret i32 [[A]] +; +entry: + br i1 %c0, label %bb_freeze, label %bb_other +bb_freeze: + %f = freeze i32 undef + br label %mid +bb_other: + br i1 %c1, label %cA, label %cB +cA: + br label %mid +cB: + br label %mid +mid: + %phi_fold = phi i32 [ %f, %bb_freeze ], [ 1, %cA ], [ 1, %cB ] + %phi_nofld = phi i32 [ %f, %bb_freeze ], [ 3, %cA ], [ 2, %cB ] + %a = add i32 %phi_fold, %phi_nofld + ret i32 %a +} + +define i32 @multi_use_one_folds_one_not_zero(i1 %c0, i1 %c1, i1 %c2) { +; CHECK-LABEL: define i32 @multi_use_one_folds_one_not_zero( +; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]], i1 [[C2:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C0]], label %[[BB_OTHER3:.*]], label %[[CC1:.*]] +; CHECK: [[BB_OTHER3]]: +; CHECK-NEXT: br label %[[MID:.*]] +; CHECK: [[CC1]]: +; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] +; CHECK: [[CA]]: +; CHECK-NEXT: br label %[[MID]] +; CHECK: [[CB]]: +; CHECK-NEXT: br label %[[MID]] +; CHECK: [[MID]]: +; CHECK-NEXT: [[PHI_FOLD:%.*]] = phi i32 [ 0, %[[BB_OTHER3]] ], [ 1, %[[CA]] ], [ 1, %[[CB]] ] +; CHECK-NEXT: br i1 [[C2]], label %[[BB_FREEZE2:.*]], label %[[CD:.*]] +; CHECK: [[BB_FREEZE2]]: +; CHECK-NEXT: br label %[[FINAL:.*]] +; CHECK: [[BB_OTHER2:.*:]] +; CHECK-NEXT: br i1 true, label %[[CA]], label %[[CB]] +; CHECK: [[CC:.*:]] +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[CD]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[FINAL]]: +; CHECK-NEXT: ret i32 [[PHI_FOLD]] +; +entry: + %f = freeze i32 undef + br i1 %c0, label %bb_freeze, label %bb_other +bb_freeze: + br label %mid +bb_other: + br i1 %c1, label %cA, label %cB +cA: + br label %mid +cB: + br label %mid +mid: + %phi_no_fold = phi i32 [ %f, %bb_freeze ], [ 1, %cA ], [ 1, %cB ] + br i1 %c2, label %bb_freeze2, label %cD +bb_freeze2: + br label %final +bb_other2: + br i1 %c1, label %cA, label %cB +cC: + br label %final +cD: + br label %final +final: + %phi_fold = phi i32 [ %f, %bb_freeze2 ], [ 0, %cC ], [ 0, %cD ] + %a = add i32 %phi_fold, %phi_no_fold + ret i32 %a +} From 0f5ed03f49805e2eb7aa808c0be0b19a2f9a3535 Mon Sep 17 00:00:00 2001 From: Mikael Nilsson Date: Mon, 29 Sep 2025 17:06:11 +0200 Subject: [PATCH 2/5] Review fix 1 Change-Id: I40d22b4b8f34bcc12b2d5f76784dd3c42d00422d --- .../InstCombine/InstructionCombining.cpp | 30 ++--- .../Transforms/InstCombine/in-freeze-phi.ll | 117 ++++++++++++------ 2 files changed, 85 insertions(+), 62 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 8e4bb38740117..646df40037ee1 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -5169,6 +5169,7 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { // - or: pick -1 // - select's condition: if the true value is constant, choose it by making // the condition true. + // - phi: pick the common constant across operands // - default: pick 0 // // Note that this transform is intentionally done here rather than @@ -5193,6 +5194,9 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { C = cast(V); + if (isa(C) || isa(C)) + return nullptr; + if (BestValue && BestValue != C) return nullptr; @@ -5203,28 +5207,8 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { Value *NullValue = Constant::getNullValue(Ty); - bool OnlyPHIUsers = - all_of(I.users(), [](const User *U) { return isa(U); }); - if (OnlyPHIUsers) { - Value *BestValue = nullptr; - for (auto *U : I.users()) { - Value *V = pickCommonConstantFromPHI(*cast(U)); - if (!V) - continue; - - if (!BestValue) { - BestValue = V; - } else if (BestValue && BestValue == NullValue) { - BestValue = NullValue; - } - } - - if (BestValue) - return BestValue; - } - Value *BestValue = nullptr; - for (const auto *U : I.users()) { + for (auto *U : I.users()) { Value *V = NullValue; if (match(U, m_Or(m_Value(), m_Value()))) V = ConstantInt::getAllOnesValue(Ty); @@ -5233,6 +5217,10 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { else if (match(U, m_c_Select(m_Specific(&I), m_Value(V)))) { if (!isGuaranteedNotToBeUndefOrPoison(V, &AC, &I, &DT)) V = NullValue; + } else if (auto *PHI = dyn_cast(U)) { + Value *MaybeV = pickCommonConstantFromPHI(*PHI); + if (MaybeV) + V = MaybeV; } if (!BestValue) diff --git a/llvm/test/Transforms/InstCombine/in-freeze-phi.ll b/llvm/test/Transforms/InstCombine/in-freeze-phi.ll index 261aafa179cb7..95641ded4fc76 100644 --- a/llvm/test/Transforms/InstCombine/in-freeze-phi.ll +++ b/llvm/test/Transforms/InstCombine/in-freeze-phi.ll @@ -147,62 +147,27 @@ final: ret <4 x i8> %phi } -define i32 @multi_use_one_folds_one_not(i1 %c0, i1 %c1) { -; CHECK-LABEL: define i32 @multi_use_one_folds_one_not( -; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]]) { -; CHECK-NEXT: [[ENTRY:.*:]] -; CHECK-NEXT: br i1 [[C0]], label %[[BB_FREEZE:.*]], label %[[BB_OTHER:.*]] -; CHECK: [[BB_FREEZE]]: -; CHECK-NEXT: br label %[[MID:.*]] -; CHECK: [[BB_OTHER]]: -; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] -; CHECK: [[CA]]: -; CHECK-NEXT: br label %[[MID]] -; CHECK: [[CB]]: -; CHECK-NEXT: br label %[[MID]] -; CHECK: [[MID]]: -; CHECK-NEXT: [[A:%.*]] = phi i32 [ 2, %[[BB_FREEZE]] ], [ 4, %[[CA]] ], [ 3, %[[CB]] ] -; CHECK-NEXT: ret i32 [[A]] -; -entry: - br i1 %c0, label %bb_freeze, label %bb_other -bb_freeze: - %f = freeze i32 undef - br label %mid -bb_other: - br i1 %c1, label %cA, label %cB -cA: - br label %mid -cB: - br label %mid -mid: - %phi_fold = phi i32 [ %f, %bb_freeze ], [ 1, %cA ], [ 1, %cB ] - %phi_nofld = phi i32 [ %f, %bb_freeze ], [ 3, %cA ], [ 2, %cB ] - %a = add i32 %phi_fold, %phi_nofld - ret i32 %a -} - define i32 @multi_use_one_folds_one_not_zero(i1 %c0, i1 %c1, i1 %c2) { ; CHECK-LABEL: define i32 @multi_use_one_folds_one_not_zero( ; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]], i1 [[C2:%.*]]) { ; CHECK-NEXT: [[ENTRY:.*:]] -; CHECK-NEXT: br i1 [[C0]], label %[[BB_OTHER3:.*]], label %[[CC1:.*]] -; CHECK: [[BB_OTHER3]]: +; CHECK-NEXT: br i1 [[C0]], label %[[BB_OTHER2:.*]], label %[[CC:.*]] +; CHECK: [[BB_OTHER2]]: ; CHECK-NEXT: br label %[[MID:.*]] -; CHECK: [[CC1]]: +; CHECK: [[CC]]: ; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] ; CHECK: [[CA]]: ; CHECK-NEXT: br label %[[MID]] ; CHECK: [[CB]]: ; CHECK-NEXT: br label %[[MID]] ; CHECK: [[MID]]: -; CHECK-NEXT: [[PHI_FOLD:%.*]] = phi i32 [ 0, %[[BB_OTHER3]] ], [ 1, %[[CA]] ], [ 1, %[[CB]] ] +; CHECK-NEXT: [[PHI_FOLD:%.*]] = phi i32 [ 0, %[[BB_OTHER2]] ], [ 1, %[[CA]] ], [ 1, %[[CB]] ] ; CHECK-NEXT: br i1 [[C2]], label %[[BB_FREEZE2:.*]], label %[[CD:.*]] ; CHECK: [[BB_FREEZE2]]: ; CHECK-NEXT: br label %[[FINAL:.*]] -; CHECK: [[BB_OTHER2:.*:]] +; CHECK: [[BB_OTHER3:.*:]] ; CHECK-NEXT: br i1 true, label %[[CA]], label %[[CB]] -; CHECK: [[CC:.*:]] +; CHECK: [[CC1:.*:]] ; CHECK-NEXT: br label %[[FINAL]] ; CHECK: [[CD]]: ; CHECK-NEXT: br label %[[FINAL]] @@ -236,3 +201,73 @@ final: %a = add i32 %phi_fold, %phi_no_fold ret i32 %a } + +define i32 @phi_freeze_poison(i1 %c0, i1 %c1) { +; CHECK-LABEL: define i32 @phi_freeze_poison( +; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C0]], label %[[BB_FREEZE:.*]], label %[[BB_OTHER:.*]] +; CHECK: [[BB_FREEZE]]: +; CHECK-NEXT: br label %[[FINAL:.*]] +; CHECK: [[BB_OTHER]]: +; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] +; CHECK: [[CA]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[CB]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[FINAL]]: +; CHECK-NEXT: ret i32 0 +; +entry: + br i1 %c0, label %bb_freeze, label %bb_other + +bb_freeze: + %f = freeze i32 undef + br label %final + +bb_other: + br i1 %c1, label %cA, label %cB +cA: + br label %final +cB: + br label %final + +final: + %phi = phi i32 [ %f, %bb_freeze ], [ poison, %cA ], [ poison, %cB ] + ret i32 %phi +} + +define i32 @phi_freeze_undef(i1 %c0, i1 %c1) { +; CHECK-LABEL: define i32 @phi_freeze_undef( +; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C0]], label %[[BB_FREEZE:.*]], label %[[BB_OTHER:.*]] +; CHECK: [[BB_FREEZE]]: +; CHECK-NEXT: br label %[[FINAL:.*]] +; CHECK: [[BB_OTHER]]: +; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] +; CHECK: [[CA]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[CB]]: +; CHECK-NEXT: br label %[[FINAL]] +; CHECK: [[FINAL]]: +; CHECK-NEXT: ret i32 0 +; +entry: + br i1 %c0, label %bb_freeze, label %bb_other + +bb_freeze: + %f = freeze i32 undef + br label %final + +bb_other: + br i1 %c1, label %cA, label %cB +cA: + br label %final +cB: + br label %final + +final: + %phi = phi i32 [ %f, %bb_freeze ], [ undef, %cA ], [ undef, %cB ] + ret i32 %phi +} From e5ad36cebf6b645cb486842c5bd5e7784a238865 Mon Sep 17 00:00:00 2001 From: Mikael Nilsson Date: Mon, 29 Sep 2025 20:07:46 +0200 Subject: [PATCH 3/5] Review fix 2 Change-Id: I1e2f175cc22c3e0190f2e8f0ad8196a35e53af71 --- .../InstCombine/InstructionCombining.cpp | 8 +++--- .../Transforms/InstCombine/in-freeze-phi.ll | 25 ++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 646df40037ee1..8d4afc58816c1 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -5180,7 +5180,7 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { // TODO: This could use getBinopAbsorber() / getBinopIdentity() to avoid // duplicating logic for binops at least. auto getUndefReplacement = [&](Type *Ty) { - auto pickCommonConstantFromPHI = [](PHINode &PN) -> Value * { + auto pickCommonConstantFromPHI = [&](PHINode &PN) -> Value * { // phi(freeze(undef), C, C). Choose C for freeze so the PHI can be // removed. Constant *BestValue = nullptr; @@ -5192,11 +5192,11 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { if (!isa(V)) return nullptr; - C = cast(V); - - if (isa(C) || isa(C)) + if (!isGuaranteedNotToBeUndefOrPoison(V, &AC, &I, &DT)) return nullptr; + C = cast(V); + if (BestValue && BestValue != C) return nullptr; diff --git a/llvm/test/Transforms/InstCombine/in-freeze-phi.ll b/llvm/test/Transforms/InstCombine/in-freeze-phi.ll index 95641ded4fc76..917d81b499c49 100644 --- a/llvm/test/Transforms/InstCombine/in-freeze-phi.ll +++ b/llvm/test/Transforms/InstCombine/in-freeze-phi.ll @@ -151,23 +151,23 @@ define i32 @multi_use_one_folds_one_not_zero(i1 %c0, i1 %c1, i1 %c2) { ; CHECK-LABEL: define i32 @multi_use_one_folds_one_not_zero( ; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]], i1 [[C2:%.*]]) { ; CHECK-NEXT: [[ENTRY:.*:]] -; CHECK-NEXT: br i1 [[C0]], label %[[BB_OTHER2:.*]], label %[[CC:.*]] -; CHECK: [[BB_OTHER2]]: +; CHECK-NEXT: br i1 [[C0]], label %[[BB_OTHER3:.*]], label %[[CC1:.*]] +; CHECK: [[BB_OTHER3]]: ; CHECK-NEXT: br label %[[MID:.*]] -; CHECK: [[CC]]: +; CHECK: [[CC1]]: ; CHECK-NEXT: br i1 [[C1]], label %[[CA:.*]], label %[[CB:.*]] ; CHECK: [[CA]]: ; CHECK-NEXT: br label %[[MID]] ; CHECK: [[CB]]: ; CHECK-NEXT: br label %[[MID]] ; CHECK: [[MID]]: -; CHECK-NEXT: [[PHI_FOLD:%.*]] = phi i32 [ 0, %[[BB_OTHER2]] ], [ 1, %[[CA]] ], [ 1, %[[CB]] ] +; CHECK-NEXT: [[PHI_FOLD:%.*]] = phi i32 [ 0, %[[BB_OTHER3]] ], [ 1, %[[CA]] ], [ 1, %[[CB]] ] ; CHECK-NEXT: br i1 [[C2]], label %[[BB_FREEZE2:.*]], label %[[CD:.*]] ; CHECK: [[BB_FREEZE2]]: ; CHECK-NEXT: br label %[[FINAL:.*]] -; CHECK: [[BB_OTHER3:.*:]] +; CHECK: [[BB_OTHER2:.*:]] ; CHECK-NEXT: br i1 true, label %[[CA]], label %[[CB]] -; CHECK: [[CC1:.*:]] +; CHECK: [[CC:.*:]] ; CHECK-NEXT: br label %[[FINAL]] ; CHECK: [[CD]]: ; CHECK-NEXT: br label %[[FINAL]] @@ -237,8 +237,8 @@ final: ret i32 %phi } -define i32 @phi_freeze_undef(i1 %c0, i1 %c1) { -; CHECK-LABEL: define i32 @phi_freeze_undef( +define <2 x i32> @phi_freeze_poison_vec(i1 %c0, i1 %c1) { +; CHECK-LABEL: define <2 x i32> @phi_freeze_poison_vec( ; CHECK-SAME: i1 [[C0:%.*]], i1 [[C1:%.*]]) { ; CHECK-NEXT: [[ENTRY:.*:]] ; CHECK-NEXT: br i1 [[C0]], label %[[BB_FREEZE:.*]], label %[[BB_OTHER:.*]] @@ -251,13 +251,14 @@ define i32 @phi_freeze_undef(i1 %c0, i1 %c1) { ; CHECK: [[CB]]: ; CHECK-NEXT: br label %[[FINAL]] ; CHECK: [[FINAL]]: -; CHECK-NEXT: ret i32 0 +; CHECK-NEXT: [[PHI:%.*]] = phi <2 x i32> [ zeroinitializer, %[[BB_FREEZE]] ], [ , %[[CA]] ], [ , %[[CB]] ] +; CHECK-NEXT: ret <2 x i32> [[PHI]] ; entry: br i1 %c0, label %bb_freeze, label %bb_other bb_freeze: - %f = freeze i32 undef + %f = freeze <2 x i32> undef br label %final bb_other: @@ -268,6 +269,6 @@ cB: br label %final final: - %phi = phi i32 [ %f, %bb_freeze ], [ undef, %cA ], [ undef, %cB ] - ret i32 %phi + %phi = phi <2 x i32> [ %f, %bb_freeze ], [ , %cA ], [ , %cB ] + ret <2 x i32> %phi } From 453332d7a306ca70eba897653a50679dc2117d1c Mon Sep 17 00:00:00 2001 From: Mikael Nilsson Date: Wed, 1 Oct 2025 08:20:45 +0200 Subject: [PATCH 4/5] Review fix 3 Change-Id: If87e6cbe36d008805ccc14e77660d245eaa64ed9 --- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 8d4afc58816c1..f6d8e928043f7 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -5180,7 +5180,7 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { // TODO: This could use getBinopAbsorber() / getBinopIdentity() to avoid // duplicating logic for binops at least. auto getUndefReplacement = [&](Type *Ty) { - auto pickCommonConstantFromPHI = [&](PHINode &PN) -> Value * { + auto pickCommonConstantFromPHI = [](PHINode &PN) -> Value * { // phi(freeze(undef), C, C). Choose C for freeze so the PHI can be // removed. Constant *BestValue = nullptr; @@ -5192,7 +5192,7 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { if (!isa(V)) return nullptr; - if (!isGuaranteedNotToBeUndefOrPoison(V, &AC, &I, &DT)) + if (!isGuaranteedNotToBeUndefOrPoison(V)) return nullptr; C = cast(V); From fe3b3abd33a7c5379c1fcd1e7c97e761fd620cd9 Mon Sep 17 00:00:00 2001 From: Mikael Nilsson Date: Wed, 1 Oct 2025 09:23:30 +0200 Subject: [PATCH 5/5] Review fix 4 Change-Id: I97f0ba5e8c41518f5cdf1322cfb5e3ec6681025e --- .../Transforms/InstCombine/InstructionCombining.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index f6d8e928043f7..0347e1748340a 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -5184,19 +5184,17 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { // phi(freeze(undef), C, C). Choose C for freeze so the PHI can be // removed. Constant *BestValue = nullptr; - Constant *C = nullptr; for (Value *V : PN.incoming_values()) { if (match(V, m_Freeze(m_Undef()))) continue; - if (!isa(V)) + Constant *C = dyn_cast(V); + if (!C) return nullptr; - if (!isGuaranteedNotToBeUndefOrPoison(V)) + if (!isGuaranteedNotToBeUndefOrPoison(C)) return nullptr; - C = cast(V); - if (BestValue && BestValue != C) return nullptr; @@ -5206,7 +5204,6 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { }; Value *NullValue = Constant::getNullValue(Ty); - Value *BestValue = nullptr; for (auto *U : I.users()) { Value *V = NullValue; @@ -5218,8 +5215,7 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { if (!isGuaranteedNotToBeUndefOrPoison(V, &AC, &I, &DT)) V = NullValue; } else if (auto *PHI = dyn_cast(U)) { - Value *MaybeV = pickCommonConstantFromPHI(*PHI); - if (MaybeV) + if (Value *MaybeV = pickCommonConstantFromPHI(*PHI)) V = MaybeV; }