-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
[DFAJumpThreading] Handle circular determinator #78177
Conversation
@llvm/pr-subscribers-llvm-transforms Author: XChy (XChy) ChangesFixes the buildbot failure in #78134 (comment) Full diff: https://github.com/llvm/llvm-project/pull/78177.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp b/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp
index 0219f65618d8d8..85d4065286e41f 100644
--- a/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp
@@ -910,8 +910,9 @@ struct TransformDFA {
PathBBs.pop_front();
auto DetIt = llvm::find(PathBBs, Determinator);
- auto Prev = std::prev(DetIt);
- BasicBlock *PrevBB = *Prev;
+ // When there is only one BB in PathBBs, the determinator takes itself as a
+ // direct predecessor.
+ BasicBlock *PrevBB = PathBBs.size() == 1 ? *DetIt : *std::prev(DetIt);
for (auto BBIt = DetIt; BBIt != PathBBs.end(); BBIt++) {
BasicBlock *BB = *BBIt;
BlocksToClean.insert(BB);
diff --git a/llvm/test/Transforms/DFAJumpThreading/dfa-jump-threading-transform.ll b/llvm/test/Transforms/DFAJumpThreading/dfa-jump-threading-transform.ll
index f40d4853d8a2fb..7cc82386aa1714 100644
--- a/llvm/test/Transforms/DFAJumpThreading/dfa-jump-threading-transform.ll
+++ b/llvm/test/Transforms/DFAJumpThreading/dfa-jump-threading-transform.ll
@@ -267,3 +267,38 @@ entry:
end:
ret void
}
+
+define void @self-reference(i1 %c) {
+; CHECK-LABEL: @self-reference(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[C:%.*]], label [[DOTSPLIT_PREHEADER:%.*]], label [[DOTSPLIT_PREHEADER]]
+; CHECK: .split.preheader:
+; CHECK-NEXT: br label [[DOTSPLIT:%.*]]
+; CHECK: .split:
+; CHECK-NEXT: [[TMP0:%.*]] = phi i32 [ 0, [[DOTSPLIT_PREHEADER]] ]
+; CHECK-NEXT: switch i32 [[TMP0]], label [[END:%.*]] [
+; CHECK-NEXT: i32 -1, label [[END]]
+; CHECK-NEXT: i32 0, label [[DOTSPLIT_JT4294967295:%.*]]
+; CHECK-NEXT: ]
+; CHECK: .split.jt4294967295:
+; CHECK-NEXT: [[TMP1:%.*]] = phi i32 [ -1, [[DOTSPLIT]] ]
+; CHECK-NEXT: br label [[END]]
+; CHECK: end:
+; CHECK-NEXT: ret void
+;
+entry:
+ br i1 %c, label %.split.preheader, label %.split.preheader
+
+.split.preheader:
+ br label %.split
+
+.split:
+ %0 = phi i32 [ 0, %.split.preheader ], [ -1, %.split ]
+ switch i32 %0, label %end [
+ i32 -1, label %end
+ i32 0, label %.split
+ ]
+
+end:
+ ret void
+}
|
I've checked this PR - it indeed fixes the asan failure in our setup as well. Would anyone mind merging if I merge it? |
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
; CHECK-NEXT: ret void | ||
; | ||
entry: | ||
br i1 %c, label %.split.preheader, label %.split.preheader |
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 this split preheader somehow relevant?
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.
43414e7 reduces it now.
I am going to merge to fix the bot. |
Fixes the buildbot failure in llvm#78134 (comment) When we meet the path with single `determinator`, the determinator actually takes itself as a predecessor. Thus, we need to let `Prev` be the determinator when `PathBBs` has only one element.
Fixes the buildbot failure in #78134 (comment)
When we meet the path with single
determinator
, the determinator actually takes itself as a predecessor. Thus, we need to letPrev
be the determinator whenPathBBs
has only one element.