-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[InstCombine] Don't check uses of constant exprs #113684
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
@llvm/pr-subscribers-llvm-transforms Author: Yingwei Zheng (dtcxzyw) ChangesAddress comment #105510 (comment). Full diff: https://github.com/llvm/llvm-project/pull/113684.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index c8b9f166b16020..60be087430be0a 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3753,7 +3753,7 @@ Instruction *InstCombinerImpl::visitBranchInst(BranchInst &BI) {
}
// Replace all dominated uses of the condition with true/false
- if (BI.getSuccessor(0) != BI.getSuccessor(1)) {
+ if (!isa<Constant>(Cond) && BI.getSuccessor(0) != BI.getSuccessor(1)) {
for (auto &U : make_early_inc_range(Cond->uses())) {
BasicBlockEdge Edge0(BI.getParent(), BI.getSuccessor(0));
if (DT.dominates(Edge0, U)) {
diff --git a/llvm/test/Transforms/InstCombine/pr105510.ll b/llvm/test/Transforms/InstCombine/pr105510.ll
new file mode 100644
index 00000000000000..844fa14ad991ee
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/pr105510.ll
@@ -0,0 +1,43 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+; Make sure we don't crash in this case.
+@g = global i32 0
+
+define i1 @foo() {
+; CHECK-LABEL: define i1 @foo() {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br i1 ptrtoint (ptr @g to i1), label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]]
+; CHECK: [[IF_THEN]]:
+; CHECK-NEXT: ret i1 true
+; CHECK: [[IF_ELSE]]:
+; CHECK-NEXT: ret i1 false
+;
+entry:
+ br i1 ptrtoint (ptr @g to i1), label %if.then, label %if.else
+
+if.then:
+ ret i1 true
+
+if.else:
+ ret i1 false
+}
+
+define i1 @bar() {
+; CHECK-LABEL: define i1 @bar() {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br i1 ptrtoint (ptr @g to i1), label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]]
+; CHECK: [[IF_THEN]]:
+; CHECK-NEXT: ret i1 true
+; CHECK: [[IF_ELSE]]:
+; CHECK-NEXT: ret i1 false
+;
+entry:
+ br i1 ptrtoint (ptr @g to i1), label %if.then, label %if.else
+
+if.then:
+ ret i1 true
+
+if.else:
+ ret i1 false
+}
|
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.
I've verified that this solves the problem I saw.
Looks good to mne, but I'm not very familiar with this so please wait for someone else to accept it.
Thanks!
I'm happy with this. Thanks again! |
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 although I think the commit message is extremely vague.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/3270 Here is the relevant piece of the build log for the reference
|
This patch skips constant expressions to avoid iterating over uses on other functions. Fix crash reported in llvm#105510 (comment).
This patch skips constant expressions to avoid iterating over uses on other functions.
Fix crash reported in #105510 (comment).