-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[InstCombine] Opt phi(freeze(undef), C) -> phi(C, C) #161181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Change-Id: I29fd2663a5c0e4e3a6d98a2c9536f515a12947a5
@llvm/pr-subscribers-llvm-transforms Author: None (mikael-nilsson-arm) ChangesChange-Id: I29fd2663a5c0e4e3a6d98a2c9536f515a12947a5 Full diff: https://github.com/llvm/llvm-project/pull/161181.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index f0ddd5ca94c5a..8e4bb38740117 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -5179,8 +5179,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<Constant>(V))
+ return nullptr;
+
+ C = cast<Constant>(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<PHINode>(U); });
+ if (OnlyPHIUsers) {
+ Value *BestValue = nullptr;
+ for (auto *U : I.users()) {
+ Value *V = pickCommonConstantFromPHI(*cast<PHINode>(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 ],
+ [<i8 9, i8 9, i8 9, i8 9>, %cA ],
+ [<i8 9, i8 9, i8 9, i8 9>, %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
+}
|
Alive proof:
|
Change-Id: I40d22b4b8f34bcc12b2d5f76784dd3c42d00422d
✅ With the latest revision this PR passed the undef deprecator. |
Change-Id: I1e2f175cc22c3e0190f2e8f0ad8196a35e53af71
Constant *BestValue = nullptr; | ||
Constant *C = nullptr; | ||
for (Value *V : PN.incoming_values()) { | ||
if (match(V, m_Freeze(m_Undef()))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it still valid if the incoming value is another freeze undef? I have not found a counterexample yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is fine. After all, we're not actually replacing that other freeze undef here. (If we did, we'd have to be more careful about multi-use.)
if (!isa<Constant>(V)) | ||
return nullptr; | ||
|
||
if (!isGuaranteedNotToBeUndefOrPoison(V, &AC, &I, &DT)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!isGuaranteedNotToBeUndefOrPoison(V, &AC, &I, &DT)) | |
if (!isGuaranteedNotToBeUndefOrPoison(V)) |
Context information is unused for constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth noting that we can also do this for non-constants, though that needs a domination check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Change-Id: If87e6cbe36d008805ccc14e77660d245eaa64ed9
// phi(freeze(undef), C, C). Choose C for freeze so the PHI can be | ||
// removed. | ||
Constant *BestValue = nullptr; | ||
Constant *C = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C is not used across iterations, so please combine it with the initialization in the loop.
if (match(V, m_Freeze(m_Undef()))) | ||
continue; | ||
|
||
if (!isa<Constant>(V)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer dyn_cast over isa + cast?
Value *NullValue = Constant::getNullValue(Ty); | ||
for (const auto *U : I.users()) { | ||
|
||
Value *BestValue = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is used inside the loop below, it was part of the original code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like I have added newline between NullValue and BestValue, will undo it.
V = NullValue; | ||
} else if (auto *PHI = dyn_cast<PHINode>(U)) { | ||
Value *MaybeV = pickCommonConstantFromPHI(*PHI); | ||
if (MaybeV) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can combine the if + variable declaration.
Change-Id: I97f0ba5e8c41518f5cdf1322cfb5e3ec6681025e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Can you please add a description to the PR? |
I updated the description. |
Try to choose a value for freeze that enables the PHI to be replaced with its input constants if they are equal.
Try to choose a value for freeze that enables the PHI to be replaced with its input constants if they are equal.