-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[DAGCombiner] visitFREEZE: Early exit when N is deleted #128161
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-backend-x86 @llvm/pr-subscribers-llvm-selectiondag Author: Yingwei Zheng (dtcxzyw) Changes
Closes #128143. Full diff: https://github.com/llvm/llvm-project/pull/128161.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index b07f3814d9d2d..3bd683fcce1e2 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -16158,11 +16158,11 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
DAG.UpdateNodeOperands(FrozenMaybePoisonOperand.getNode(),
MaybePoisonOperand);
}
- }
- // This node has been merged with another.
- if (N->getOpcode() == ISD::DELETED_NODE)
- return SDValue(N, 0);
+ // This node has been merged with another.
+ if (N->getOpcode() == ISD::DELETED_NODE)
+ return SDValue(N, 0);
+ }
// The whole node may have been updated, so the value we were holding
// may no longer be valid. Re-fetch the operand we're `freeze`ing.
diff --git a/llvm/test/CodeGen/X86/pr128143.ll b/llvm/test/CodeGen/X86/pr128143.ll
new file mode 100644
index 0000000000000..2517ad9ebcb6b
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr128143.ll
@@ -0,0 +1,32 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s
+
+@g_1 = external global i8
+@g_2 = external global i8
+
+; Make sure we don't crash on this test.
+
+define i1 @test(i1 %cmp1, i32 %x) {
+; CHECK-LABEL: test:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: movq g_2@GOTPCREL(%rip), %rcx
+; CHECK-NEXT: xorl %eax, %eax
+; CHECK-NEXT: cmpq %rcx, g_1@GOTPCREL(%rip)
+; CHECK-NEXT: setne %al
+; CHECK-NEXT: cmpl %eax, %esi
+; CHECK-NEXT: setb %cl
+; CHECK-NEXT: orb %cl, %al
+; CHECK-NEXT: andb %dil, %al
+; CHECK-NEXT: # kill: def $al killed $al killed $eax
+; CHECK-NEXT: retq
+entry:
+ %cmp2 = icmp ne ptr @g_1, @g_2
+ %fr = freeze ptr @g_1
+ %cmp3 = icmp ne ptr %fr, @g_2
+ %ext1 = zext i1 %cmp3 to i32
+ %sel1 = select i1 %cmp1, i1 %cmp2, i1 false
+ %cmp4 = icmp ult i32 %x, %ext1
+ %sel3 = select i1 %cmp1, i1 %cmp4, i1 false
+ %or = or i1 %sel1, %sel3
+ ret i1 %or
+}
|
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 with one optional minor
/cherry-pick 646e4f2 |
/pull-request #128283 |
`N` may get merged with existing nodes inside the loop. Early exit when it is deleted to avoid the crash. Alternative solution: use `DAGNodeDeletedListener` to refresh the value of N. Closes llvm#128143. (cherry picked from commit 646e4f2)
N
may get merged with existing nodes inside the loop. Early exit when it is deleted to avoid the crash.Alternative solution: use
DAGNodeDeletedListener
to refresh the value of N.Closes #128143.