Skip to content

Conversation

@Saldivarcher
Copy link
Contributor

@Saldivarcher Saldivarcher commented Oct 22, 2024

Looks like having a constant in Z also caused infinite loops. This fixes #113240.

@llvmbot
Copy link
Member

llvmbot commented Oct 22, 2024

@llvm/pr-subscribers-backend-x86

Author: Miguel Saldivar (Saldivarcher)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/113264.diff

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+2-2)
  • (modified) llvm/test/CodeGen/X86/pr108731.ll (+20)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index bcb84add65d83e..5ae5a7dbe0f6da 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -50049,8 +50049,8 @@ static SDValue combineAndNotOrIntoAndNotAnd(SDNode *N, SelectionDAG &DAG) {
   SDValue X, Y, Z;
   if (sd_match(N, m_And(m_Value(X),
                         m_OneUse(m_Or(m_Value(Y), m_Not(m_Value(Z))))))) {
-    // Don't fold if Y is a constant to prevent infinite loops.
-    if (!isa<ConstantSDNode>(Y))
+    // Don't fold if Y and Z are constants to prevent infinite loops.
+    if (!isa<ConstantSDNode>(Y) && !isa<ConstantSDNode>(Z))
       return DAG.getNode(
           ISD::AND, DL, VT, X,
           DAG.getNOT(
diff --git a/llvm/test/CodeGen/X86/pr108731.ll b/llvm/test/CodeGen/X86/pr108731.ll
index 473b4f7f4da2e3..2983d108eaeddc 100644
--- a/llvm/test/CodeGen/X86/pr108731.ll
+++ b/llvm/test/CodeGen/X86/pr108731.ll
@@ -192,3 +192,23 @@ define void @PR112347(ptr %p0, ptr %p1, ptr %p2) {
   ret void
 }
 
+define void @PR113240(i64 %a) {
+; CHECK-LABEL: PR113240:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    movq %rdi, %rax
+; CHECK-NEXT:    notq %rax
+; CHECK-NEXT:    movabsq $8796093022206, %rcx # imm = 0x7FFFFFFFFFE
+; CHECK-NEXT:    notq %rcx
+; CHECK-NEXT:    orq %rax, %rcx
+; CHECK-NEXT:    andq %rdi, %rcx
+; CHECK-NEXT:    movq %rcx, 0
+; CHECK-NEXT:    retq
+entry:
+  %and = and i64 %a, 8796093022206
+  %bf.value = and i64 8796093022206, 0
+  %not = xor i64 %and, -1
+  %and4 = and i64 %a, %not
+  store i64 %and4, ptr null, align 8
+  ret void
+}
+

@Saldivarcher
Copy link
Contributor Author

@RKSimon let me know what you think.

@Saldivarcher
Copy link
Contributor Author

@phoebewang and @goldsteinn would you be able to review this? @RKSimon might be away.

@RKSimon
Copy link
Collaborator

RKSimon commented Oct 22, 2024

Sorry, been travelling for the devmtg

@Saldivarcher
Copy link
Contributor Author

Sorry, been travelling for the devmtg

It's ok, sorry for bugging you! Safe travels btw!

Copy link
Contributor

@goldsteinn goldsteinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although this type of re-ordering is liable to creating inf loops and this might be the only case. If this continues to be an issue we should probably drop the fold.

@Saldivarcher
Copy link
Contributor Author

@goldsteinn would you be able to rebase and merge for me? I don't have write access. I'd be ok with reverting if this keeps causing issues.

Thanks for taking a look btw. 👍

@phoebewang phoebewang merged commit 49ebe32 into llvm:main Oct 23, 2024
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compiler in infinite loop after 6fd229a

5 participants