[llvm] [InstSimplify] Fix Inconsistent PHI Simplification (PR #113037)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 19 06:10:58 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-analysis
Author: Marius Kamp (mskamp)
<details>
<summary>Changes</summary>
Before this change, the simplification of PHI functions was inconsistent with
the simplification of `select` instructions. The simplification of `select`
instructions includes special handling of vector operands, which the
simplification of PHI functions lacked. This resulted in different
simplifications for partially `undef` vectors.
This change extends the simplification of PHI functions by handling also vector
operands that are partially `undef` or `poison`. As is done for `select`
operations, combining `undef` and `poison` elements now yields an `undef`
element.
This is consistent with the reasoning in Alive:
https://alive2.llvm.org/ce/z/vbmYTa
Adapt also a code generation test case. This adaption is necessary to keep the
loop structure even with the changed simplification of PHI functions, which
would otherwise simplify the control flow too much.
Fixes #<!-- -->98441
---
Full diff: https://github.com/llvm/llvm-project/pull/113037.diff
3 Files Affected:
- (modified) llvm/lib/Analysis/InstructionSimplify.cpp (+54-8)
- (modified) llvm/test/CodeGen/AMDGPU/collapse-endcf.ll (+13-13)
- (added) llvm/test/Transforms/InstSimplify/phi-vec-unknown-poison.ll (+117)
``````````diff
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index d08be1e55c853e..694138c93d6f12 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -5288,6 +5288,54 @@ Value *llvm::simplifyExtractElementInst(Value *Vec, Value *Idx,
return ::simplifyExtractElementInst(Vec, Idx, Q, RecursionLimit);
}
+static Value *getCommonPHIValue(Value *PreviousCommon, Value *Current,
+ const SimplifyQuery &Q) {
+ if (!PreviousCommon)
+ return Current;
+
+ if (PreviousCommon == Current)
+ return PreviousCommon;
+
+ Constant *PreviousCommonC, *CurrentC;
+ if (match(PreviousCommon, m_Constant(PreviousCommonC)) &&
+ match(Current, m_Constant(CurrentC))) {
+ auto *VecTy = dyn_cast<FixedVectorType>(PreviousCommonC->getType());
+ if (VecTy) {
+ SmallVector<Constant *> NewCommonC;
+ unsigned NumElts = VecTy->getNumElements();
+ for (unsigned I = 0; I != NumElts; ++I) {
+ Constant *PrevElt = PreviousCommonC->getAggregateElement(I);
+ Constant *CurrElt = CurrentC->getAggregateElement(I);
+ if (!PrevElt || !CurrElt)
+ return nullptr;
+
+ if (PrevElt == CurrElt)
+ NewCommonC.push_back(PrevElt);
+ else if (isa<PoisonValue>(PrevElt) ||
+ (Q.isUndefValue(PrevElt) &&
+ isGuaranteedNotToBePoison(CurrElt)))
+ NewCommonC.push_back(CurrElt);
+ else if (isa<PoisonValue>(CurrElt) ||
+ (Q.isUndefValue(CurrElt) &&
+ isGuaranteedNotToBePoison(PrevElt)))
+ NewCommonC.push_back(PrevElt);
+ else
+ return nullptr;
+ }
+
+ return ConstantVector::get(NewCommonC);
+ }
+ }
+
+ if (Q.isUndefValue(PreviousCommon))
+ return Current;
+
+ if (Q.isUndefValue(Current))
+ return PreviousCommon;
+
+ return nullptr;
+}
+
/// See if we can fold the given phi. If not, returns null.
static Value *simplifyPHINode(PHINode *PN, ArrayRef<Value *> IncomingValues,
const SimplifyQuery &Q) {
@@ -5309,20 +5357,18 @@ static Value *simplifyPHINode(PHINode *PN, ArrayRef<Value *> IncomingValues,
continue;
}
if (Q.isUndefValue(Incoming)) {
- // Remember that we saw an undef value, but otherwise ignore them.
+ // Remember that we saw an undef value.
HasUndefInput = true;
- continue;
}
- if (CommonValue && Incoming != CommonValue)
+ CommonValue = getCommonPHIValue(CommonValue, Incoming, Q);
+ if (!CommonValue)
return nullptr; // Not the same, bail out.
- CommonValue = Incoming;
}
- // If CommonValue is null then all of the incoming values were either undef,
- // poison or equal to the phi node itself.
+ // If CommonValue is null then all of the incoming values were either poison
+ // or equal to the phi node itself.
if (!CommonValue)
- return HasUndefInput ? UndefValue::get(PN->getType())
- : PoisonValue::get(PN->getType());
+ return PoisonValue::get(PN->getType());
if (HasPoisonInput || HasUndefInput) {
// If we have a PHI node like phi(X, undef, X), where X is defined by some
diff --git a/llvm/test/CodeGen/AMDGPU/collapse-endcf.ll b/llvm/test/CodeGen/AMDGPU/collapse-endcf.ll
index 75d0b83a024ff5..5bf050f60fc010 100644
--- a/llvm/test/CodeGen/AMDGPU/collapse-endcf.ll
+++ b/llvm/test/CodeGen/AMDGPU/collapse-endcf.ll
@@ -983,7 +983,8 @@ define void @scc_liveness(i32 %arg) local_unnamed_addr #0 {
; GCN-NEXT: v_cmp_gt_i32_e32 vcc, s4, v0
; GCN-NEXT: s_mov_b32 s8, 0
; GCN-NEXT: v_cmp_eq_u32_e64 s[4:5], 0, v0
-; GCN-NEXT: s_mov_b64 s[12:13], 0
+; GCN-NEXT: s_mov_b64 s[14:15], 0
+; GCN-NEXT: s_mov_b32 s13, 1.0
; GCN-NEXT: s_mov_b64 s[6:7], 0
; GCN-NEXT: s_branch .LBB5_3
; GCN-NEXT: .LBB5_1: ; %Flow
@@ -991,11 +992,11 @@ define void @scc_liveness(i32 %arg) local_unnamed_addr #0 {
; GCN-NEXT: s_or_b64 exec, exec, s[10:11]
; GCN-NEXT: .LBB5_2: ; %bb10
; GCN-NEXT: ; in Loop: Header=BB5_3 Depth=1
-; GCN-NEXT: s_or_b64 exec, exec, s[14:15]
+; GCN-NEXT: s_or_b64 exec, exec, s[16:17]
; GCN-NEXT: s_and_b64 s[6:7], exec, s[4:5]
-; GCN-NEXT: s_or_b64 s[12:13], s[6:7], s[12:13]
+; GCN-NEXT: s_or_b64 s[14:15], s[6:7], s[14:15]
; GCN-NEXT: s_mov_b64 s[6:7], 0
-; GCN-NEXT: s_andn2_b64 exec, exec, s[12:13]
+; GCN-NEXT: s_andn2_b64 exec, exec, s[14:15]
; GCN-NEXT: s_cbranch_execz .LBB5_7
; GCN-NEXT: .LBB5_3: ; %bb1
; GCN-NEXT: ; =>This Inner Loop Header: Depth=1
@@ -1013,7 +1014,7 @@ define void @scc_liveness(i32 %arg) local_unnamed_addr #0 {
; GCN-NEXT: v_mov_b32_e32 v1, s9
; GCN-NEXT: v_mov_b32_e32 v2, s10
; GCN-NEXT: v_mov_b32_e32 v3, s11
-; GCN-NEXT: s_and_saveexec_b64 s[14:15], s[4:5]
+; GCN-NEXT: s_and_saveexec_b64 s[16:17], s[4:5]
; GCN-NEXT: s_cbranch_execz .LBB5_2
; GCN-NEXT: ; %bb.5: ; %bb4
; GCN-NEXT: ; in Loop: Header=BB5_3 Depth=1
@@ -1028,14 +1029,13 @@ define void @scc_liveness(i32 %arg) local_unnamed_addr #0 {
; GCN-NEXT: s_cbranch_execz .LBB5_1
; GCN-NEXT: ; %bb.6: ; %bb8
; GCN-NEXT: ; in Loop: Header=BB5_3 Depth=1
-; GCN-NEXT: s_mov_b32 s9, s8
-; GCN-NEXT: v_mov_b32_e32 v0, s8
-; GCN-NEXT: v_mov_b32_e32 v1, s9
-; GCN-NEXT: v_mov_b32_e32 v2, s10
-; GCN-NEXT: v_mov_b32_e32 v3, s11
+; GCN-NEXT: v_mov_b32_e32 v0, s12
+; GCN-NEXT: v_mov_b32_e32 v1, s13
+; GCN-NEXT: v_mov_b32_e32 v2, s14
+; GCN-NEXT: v_mov_b32_e32 v3, s15
; GCN-NEXT: s_branch .LBB5_1
; GCN-NEXT: .LBB5_7: ; %bb12
-; GCN-NEXT: s_or_b64 exec, exec, s[12:13]
+; GCN-NEXT: s_or_b64 exec, exec, s[14:15]
; GCN-NEXT: buffer_store_dword v3, v0, s[0:3], 0 offen
; GCN-NEXT: s_waitcnt vmcnt(0)
; GCN-NEXT: buffer_store_dword v2, v0, s[0:3], 0 offen
@@ -1168,7 +1168,7 @@ define void @scc_liveness(i32 %arg) local_unnamed_addr #0 {
; GCN-O0-NEXT: s_cbranch_execz .LBB5_6
; GCN-O0-NEXT: ; %bb.4: ; %bb8
; GCN-O0-NEXT: ; in Loop: Header=BB5_1 Depth=1
-; GCN-O0-NEXT: s_mov_b32 s10, 0
+; GCN-O0-NEXT: s_mov_b32 s10, 1.0
; GCN-O0-NEXT: ; implicit-def: $sgpr4
; GCN-O0-NEXT: ; implicit-def: $sgpr5
; GCN-O0-NEXT: ; implicit-def: $sgpr9
@@ -1372,7 +1372,7 @@ bb4: ; preds = %bb2
br i1 %tmp7, label %bb8, label %Flow
bb8: ; preds = %bb4
- %tmp9 = insertelement <4 x float> undef, float 0.0, i32 1
+ %tmp9 = insertelement <4 x float> undef, float 1.0, i32 1
br label %Flow
Flow: ; preds = %bb8, %bb4
diff --git a/llvm/test/Transforms/InstSimplify/phi-vec-unknown-poison.ll b/llvm/test/Transforms/InstSimplify/phi-vec-unknown-poison.ll
new file mode 100644
index 00000000000000..4b84417af187c7
--- /dev/null
+++ b/llvm/test/Transforms/InstSimplify/phi-vec-unknown-poison.ll
@@ -0,0 +1,117 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
+
+define <2 x i1> @test_vec_undef_poison(i1 %c) {
+; CHECK-LABEL: define <2 x i1> @test_vec_undef_poison(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT: br i1 [[C]], label %[[T:.*]], label %[[F:.*]]
+; CHECK: [[T]]:
+; CHECK-NEXT: br label %[[M:.*]]
+; CHECK: [[F]]:
+; CHECK-NEXT: br label %[[M]]
+; CHECK: [[M]]:
+; CHECK-NEXT: ret <2 x i1> <i1 undef, i1 false>
+;
+ br i1 %c, label %t, label %f
+t:
+ br label %m
+f:
+ br label %m
+m:
+ %res = phi <2 x i1> [ <i1 undef, i1 poison>, %t ], [ <i1 poison, i1 false>, %f ]
+ ret <2 x i1> %res
+}
+
+define <2 x i1> @test_vec_poison_undef(i1 %c) {
+; CHECK-LABEL: define <2 x i1> @test_vec_poison_undef(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT: br i1 [[C]], label %[[T:.*]], label %[[F:.*]]
+; CHECK: [[T]]:
+; CHECK-NEXT: br label %[[M:.*]]
+; CHECK: [[F]]:
+; CHECK-NEXT: br label %[[M]]
+; CHECK: [[M]]:
+; CHECK-NEXT: ret <2 x i1> <i1 false, i1 undef>
+;
+ br i1 %c, label %t, label %f
+t:
+ br label %m
+f:
+ br label %m
+m:
+ %res = phi <2 x i1> [ <i1 false, i1 poison>, %t ], [ <i1 poison, i1 undef>, %f ]
+ ret <2 x i1> %res
+}
+
+define <2 x i1> @test_vec_poison_undef_defined(i1 %c1, i1 %c2) {
+; CHECK-LABEL: define <2 x i1> @test_vec_poison_undef_defined(
+; CHECK-SAME: i1 [[C1:%.*]], i1 [[C2:%.*]]) {
+; CHECK-NEXT: br i1 [[C1]], label %[[T:.*]], label %[[F:.*]]
+; CHECK: [[T]]:
+; CHECK-NEXT: br i1 [[C2]], label %[[TT:.*]], label %[[TF:.*]]
+; CHECK: [[TT]]:
+; CHECK-NEXT: br label %[[M:.*]]
+; CHECK: [[TF]]:
+; CHECK-NEXT: br label %[[M]]
+; CHECK: [[F]]:
+; CHECK-NEXT: br label %[[M]]
+; CHECK: [[M]]:
+; CHECK-NEXT: ret <2 x i1> <i1 false, i1 true>
+;
+ br i1 %c1, label %t, label %f
+t:
+ br i1 %c2, label %tt, label %tf
+tt:
+ br label %m
+tf:
+ br label %m
+f:
+ br label %m
+m:
+ %res = phi <2 x i1> [ <i1 false, i1 poison>, %tt ], [ <i1 undef, i1 undef>, %tf ], [ <i1 poison, i1 true>, %f ]
+ ret <2 x i1> %res
+}
+
+define <2 x i8> @test_vec_same_constants(i1 %c) {
+; CHECK-LABEL: define <2 x i8> @test_vec_same_constants(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT: br i1 [[C]], label %[[T:.*]], label %[[F:.*]]
+; CHECK: [[T]]:
+; CHECK-NEXT: br label %[[M:.*]]
+; CHECK: [[F]]:
+; CHECK-NEXT: br label %[[M]]
+; CHECK: [[M]]:
+; CHECK-NEXT: ret <2 x i8> <i8 4, i8 2>
+;
+ br i1 %c, label %t, label %f
+t:
+ br label %m
+f:
+ br label %m
+m:
+ %res = phi <2 x i8> [ <i8 4, i8 2>, %t ], [ <i8 4, i8 2>, %f ]
+ ret <2 x i8> %res
+}
+
+; Negative test
+define <2 x i8> @test_vec_different_constants(i1 %c) {
+; CHECK-LABEL: define <2 x i8> @test_vec_different_constants(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT: br i1 [[C]], label %[[T:.*]], label %[[F:.*]]
+; CHECK: [[T]]:
+; CHECK-NEXT: br label %[[M:.*]]
+; CHECK: [[F]]:
+; CHECK-NEXT: br label %[[M]]
+; CHECK: [[M]]:
+; CHECK-NEXT: [[RES:%.*]] = phi <2 x i8> [ <i8 4, i8 2>, %[[T]] ], [ <i8 2, i8 4>, %[[F]] ]
+; CHECK-NEXT: ret <2 x i8> [[RES]]
+;
+ br i1 %c, label %t, label %f
+t:
+ br label %m
+f:
+ br label %m
+m:
+ %res = phi <2 x i8> [ <i8 4, i8 2>, %t ], [ <i8 2, i8 4>, %f ]
+ ret <2 x i8> %res
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/113037
More information about the llvm-commits
mailing list