Skip to content

Conversation

@Saldivarcher
Copy link
Contributor

@Saldivarcher Saldivarcher commented Oct 21, 2024

Reverting this for now, until I can figure out why an infinite loop is happening.

This is the code causing the infinite loop:

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-generic-linux-gnu"

define void @_renamed(i64 %error_code.coerce) #0 {
entry:
  %0 = and i64 %error_code.coerce, 4294967296
  %1 = xor i64 %0, -1
  %2 = and i64 %error_code.coerce, %1
  %3 = icmp eq i64 %2, 0
  br i1 %3, label %5, label %4

4:                                                ; preds = %entry
  unreachable

5:                                                ; preds = %entry
  ret void
}

attributes #0 = { "target-cpu"="haswell" }

@llvmbot llvmbot added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Oct 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Author: Miguel Saldivar (Saldivarcher)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (-2)
  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (-28)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 50a75bc5932c42..91566b9040bcbc 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -9595,7 +9595,6 @@ SDValue DAGCombiner::visitXOR(SDNode *N) {
   }
 
   // fold (not (or x, y)) -> (and (not x), (not y)) iff x or y are setcc
-  // fold (not (and x, y)) -> (or (not x), (not y)) iff x or y are setcc
   if (isOneConstant(N1) && VT == MVT::i1 && N0.hasOneUse() &&
       (N0Opcode == ISD::OR || N0Opcode == ISD::AND)) {
     SDValue N00 = N0.getOperand(0), N01 = N0.getOperand(1);
@@ -9608,7 +9607,6 @@ SDValue DAGCombiner::visitXOR(SDNode *N) {
     }
   }
   // fold (not (or x, y)) -> (and (not x), (not y)) iff x or y are constants
-  // fold (not (and x, y)) -> (or (not x), (not y)) iff x or y are constants
   if (isAllOnesConstant(N1) && N0.hasOneUse() &&
       (N0Opcode == ISD::OR || N0Opcode == ISD::AND)) {
     SDValue N00 = N0.getOperand(0), N01 = N0.getOperand(1);
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index bcb84add65d83e..1b6f4fe0eec2db 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -50035,31 +50035,6 @@ static bool hasBZHI(const X86Subtarget &Subtarget, MVT VT) {
          (VT == MVT::i32 || (VT == MVT::i64 && Subtarget.is64Bit()));
 }
 
-/// Folds (and X, (or Y, ~Z)) --> (and X, ~(and ~Y, Z))
-/// This undoes the inverse fold performed in InstCombine
-static SDValue combineAndNotOrIntoAndNotAnd(SDNode *N, SelectionDAG &DAG) {
-
-  using namespace llvm::SDPatternMatch;
-  MVT VT = N->getSimpleValueType(0);
-  SDLoc DL(N);
-  const TargetLowering &TLI = DAG.getTargetLoweringInfo();
-  if (!TLI.hasAndNot(SDValue(N, 0)))
-    return SDValue();
-
-  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))
-      return DAG.getNode(
-          ISD::AND, DL, VT, X,
-          DAG.getNOT(
-              DL, DAG.getNode(ISD::AND, DL, VT, DAG.getNOT(DL, Y, VT), Z), VT));
-  }
-
-  return SDValue();
-}
-
 // This function recognizes cases where X86 bzhi instruction can replace and
 // 'and-load' sequence.
 // In case of loading integer value from an array of constants which is defined
@@ -50551,9 +50526,6 @@ static SDValue combineAnd(SDNode *N, SelectionDAG &DAG,
   if (SDValue R = combineAndLoadToBZHI(N, DAG, Subtarget))
     return R;
 
-  if (SDValue R = combineAndNotOrIntoAndNotAnd(N, DAG))
-    return R;
-
   // fold (and (mul x, c1), c2) -> (mul x, (and c1, c2))
   // iff c2 is all/no bits mask - i.e. a select-with-zero mask.
   // TODO: Handle PMULDQ/PMULUDQ/VPMADDWD/VPMADDUBSW?

@Saldivarcher
Copy link
Contributor Author

@RKSimon does this look ok?

@Saldivarcher
Copy link
Contributor Author

cc @bgra8

@Saldivarcher
Copy link
Contributor Author

Not needed looks like @RKSimon's commit also fixes this issue as well:

commit ec78f0da0e9b1b8e2b2323e434ea742e272dd913
Author: Simon Pilgrim <[email protected]>
Date:   Tue Oct 15 16:54:08 2024 +0100

    [X86] combineAndNotOrIntoAndNotAnd - don't attempt with constant operands
    
    Don't fold AND(X,OR(NOT(Z),C)) -> AND(X,NOT(AND(Z,C'))) as DAGCombiner will invert it back again.
    
    Fixes #112347

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:X86 llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants