[ValueTracking] Skip incoming values that are the same as the phi in isGuaranteedNotToBeUndefOrPoison
#130111
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-analysis Author: DianQK (DianQK) ChangesFixes #130110. If the incoming value is PHI itself, we can skip this. If we can guarantee that the other incoming values are neither undef nor poison, then we can also guarantee that the value isn't either. If we cannot guarantee that, it makes no sense in calculating it. Full diff: https://github.com/llvm/llvm-project/pull/130111.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index e3e026f7979da..9b098bcb28bcc 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -7824,6 +7824,8 @@ static bool isGuaranteedNotToBeUndefOrPoison(
unsigned Num = PN->getNumIncomingValues();
bool IsWellDefined = true;
for (unsigned i = 0; i < Num; ++i) {
+ if (PN == PN->getIncomingValue(i))
+ continue;
auto *TI = PN->getIncomingBlock(i)->getTerminator();
if (!isGuaranteedNotToBeUndefOrPoison(PN->getIncomingValue(i), AC, TI,
DT, Depth + 1, Kind)) {
diff --git a/llvm/test/Analysis/ValueTracking/phi-self.ll b/llvm/test/Analysis/ValueTracking/phi-self.ll
new file mode 100644
index 0000000000000..095a16983ce21
--- /dev/null
+++ b/llvm/test/Analysis/ValueTracking/phi-self.ll
@@ -0,0 +1,145 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=instsimplify < %s | FileCheck %s
+
+; Test `%r` can be replaced by `%nonpoison`.
+
+define i64 @other_noundef() {
+; CHECK-LABEL: define i64 @other_noundef() {
+; CHECK-NEXT: [[START:.*]]:
+; CHECK-NEXT: br label %[[LOOP:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[R:%.*]] = phi i64 [ [[R]], %[[BB0:.*]] ], [ [[R]], %[[BB1:.*]] ], [ [[R]], %[[BB2:.*]] ], [ [[R]], %[[BB:.*]] ], [ [[I:%.*]], %[[BACK_TO_LOOP:.*]] ], [ 0, %[[START]] ]
+; CHECK-NEXT: [[I]] = call i64 @opaque()
+; CHECK-NEXT: switch i64 [[I]], label %[[EXIT0:.*]] [
+; CHECK-NEXT: i64 -1, label %[[EXIT1:.*]]
+; CHECK-NEXT: i64 2, label %[[BACK_TO_LOOP]]
+; CHECK-NEXT: i64 0, label %[[BB]]
+; CHECK-NEXT: ]
+; CHECK: [[EXIT0]]:
+; CHECK-NEXT: br label %[[EXIT1]]
+; CHECK: [[EXIT1]]:
+; CHECK-NEXT: ret i64 [[R]]
+; CHECK: [[BACK_TO_LOOP]]:
+; CHECK-NEXT: br label %[[LOOP]]
+; CHECK: [[BB]]:
+; CHECK-NEXT: switch i64 [[R]], label %[[LOOP]] [
+; CHECK-NEXT: i64 0, label %[[BB0]]
+; CHECK-NEXT: i64 1, label %[[BB1]]
+; CHECK-NEXT: i64 2, label %[[BB2]]
+; CHECK-NEXT: ]
+; CHECK: [[BB0]]:
+; CHECK-NEXT: br label %[[LOOP]]
+; CHECK: [[BB1]]:
+; CHECK-NEXT: br label %[[LOOP]]
+; CHECK: [[BB2]]:
+; CHECK-NEXT: br label %[[LOOP]]
+;
+start:
+ br label %loop
+
+loop: ; preds = %bb2, %bb1, %bb0, %bb, %back_to_loop, %start
+ %nonpoison = phi i64 [ %nonpoison, %bb0 ], [ %nonpoison, %bb1 ], [ %nonpoison, %bb2 ], [ %nonpoison, %bb ], [ %i, %back_to_loop ], [ 0, %start ]
+ %i = call i64 @opaque()
+ switch i64 %i, label %exit0 [
+ i64 -1, label %exit1
+ i64 2, label %back_to_loop
+ i64 0, label %bb
+ ]
+
+exit0: ; preds = %loop
+ br label %exit1
+
+exit1: ; preds = %exit0, %loop
+ %r = phi i64 [ %nonpoison, %loop ], [ undef, %exit0 ]
+ ret i64 %r
+
+back_to_loop: ; preds = %loop
+ br label %loop
+
+bb: ; preds = %loop
+ switch i64 %nonpoison, label %loop [
+ i64 0, label %bb0
+ i64 1, label %bb1
+ i64 2, label %bb2
+ ]
+
+bb0: ; preds = %bb
+ br label %loop
+
+bb1: ; preds = %bb
+ br label %loop
+
+bb2: ; preds = %bb
+ br label %loop
+}
+
+define i64 @other_poison() {
+; CHECK-LABEL: define i64 @other_poison() {
+; CHECK-NEXT: [[START:.*:]]
+; CHECK-NEXT: br label %[[LOOP:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[I:%.*]] = call i64 @opaque()
+; CHECK-NEXT: switch i64 [[I]], label %[[EXIT0:.*]] [
+; CHECK-NEXT: i64 -1, label %[[EXIT1:.*]]
+; CHECK-NEXT: i64 2, label %[[BACK_TO_LOOP:.*]]
+; CHECK-NEXT: i64 0, label %[[BB:.*]]
+; CHECK-NEXT: ]
+; CHECK: [[EXIT0]]:
+; CHECK-NEXT: br label %[[EXIT1]]
+; CHECK: [[EXIT1]]:
+; CHECK-NEXT: ret i64 0
+; CHECK: [[BACK_TO_LOOP]]:
+; CHECK-NEXT: br label %[[LOOP]]
+; CHECK: [[BB]]:
+; CHECK-NEXT: switch i64 0, label %[[LOOP]] [
+; CHECK-NEXT: i64 0, label %[[BB0:.*]]
+; CHECK-NEXT: i64 1, label %[[BB1:.*]]
+; CHECK-NEXT: i64 2, label %[[BB2:.*]]
+; CHECK-NEXT: ]
+; CHECK: [[BB0]]:
+; CHECK-NEXT: br label %[[LOOP]]
+; CHECK: [[BB1]]:
+; CHECK-NEXT: br label %[[LOOP]]
+; CHECK: [[BB2]]:
+; CHECK-NEXT: br label %[[LOOP]]
+;
+start:
+ br label %loop
+
+loop: ; preds = %bb2, %bb1, %bb0, %bb, %back_to_loop, %start
+ %maypoison = phi i64 [ %maypoison, %bb0 ], [ %maypoison, %bb1 ], [ %maypoison, %bb2 ], [ %maypoison, %bb ], [ poison, %back_to_loop ], [ 0, %start ]
+ %i = call i64 @opaque()
+ switch i64 %i, label %exit0 [
+ i64 -1, label %exit1
+ i64 2, label %back_to_loop
+ i64 0, label %bb
+ ]
+
+exit0: ; preds = %loop
+ br label %exit1
+
+exit1: ; preds = %exit0, %loop
+ %r = phi i64 [ %maypoison, %loop ], [ undef, %exit0 ]
+ ret i64 %r
+
+back_to_loop: ; preds = %loop
+ br label %loop
+
+bb: ; preds = %loop
+ switch i64 %maypoison, label %loop [
+ i64 0, label %bb0
+ i64 1, label %bb1
+ i64 2, label %bb2
+ ]
+
+bb0: ; preds = %bb
+ br label %loop
+
+bb1: ; preds = %bb
+ br label %loop
+
+bb2: ; preds = %bb
+ br label %loop
+}
+
+declare i64 @opaque()
|
nikic
reviewed
Mar 6, 2025
nikic
reviewed
Mar 6, 2025
There was a problem hiding this comment.
The change is fine, though I wouldn't really consider it a "fix" for the compile-time issue. What we normally do is to limit recursion when analyzing phis:
llvm-project/llvm/lib/Analysis/ValueTracking.cpp
Lines 1703 to 1704 in d998eed
Ah, yes. There's a limitation here, and I think this is a special scenario where the limitation fails. I will do that. |
nikic
reviewed
Mar 6, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/9143 Here is the relevant piece of the build log for the reference |
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 12, 2025
…`isGuaranteedNotToBeUndefOrPoison` (llvm#130111) Fixes (keep it open) llvm#130110. If the incoming value is PHI itself, we can skip this. If we can guarantee that the other incoming values are neither undef nor poison, then we can also guarantee that the value isn't either. If we cannot guarantee that, it makes no sense in calculating it. (cherry picked from commit 462eb7e)
jph-13
pushed a commit
to jph-13/llvm-project
that referenced
this pull request
Mar 21, 2025
…`isGuaranteedNotToBeUndefOrPoison` (llvm#130111) Fixes (keep it open) llvm#130110. If the incoming value is PHI itself, we can skip this. If we can guarantee that the other incoming values are neither undef nor poison, then we can also guarantee that the value isn't either. If we cannot guarantee that, it makes no sense in calculating it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes (keep it open) #130110.
If the incoming value is PHI itself, we can skip this. If we can guarantee that the other incoming values are neither undef nor poison, then we can also guarantee that the value isn't either. If we cannot guarantee that, it makes no sense in calculating it.