Skip to content

Conversation

AZero13
Copy link
Contributor

@AZero13 AZero13 commented Sep 9, 2025

These transforms are now handled in DAGCombine, so enable hasAndNotCompare for the same cases for X86, and remove the platform-specific code that does the same thing.

@llvmbot
Copy link
Member

llvmbot commented Sep 9, 2025

@llvm/pr-subscribers-backend-x86

Author: AZero13 (AZero13)

Changes

These transforms are now handled in DAGCombine, so enable hasAndNotCompare for the same cases for X86, and remove the platform-specific code that does the same thing.


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

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+2-45)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 08ae0d52d795e..6dbe45105aa98 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -3491,17 +3491,11 @@ bool X86TargetLowering::isMaskAndCmp0FoldingBeneficial(
 bool X86TargetLowering::hasAndNotCompare(SDValue Y) const {
   EVT VT = Y.getValueType();
 
-  if (VT.isVector())
-    return false;
-
   if (!Subtarget.hasBMI())
     return false;
 
-  // There are only 32-bit and 64-bit forms for 'andn'.
-  if (VT != MVT::i32 && VT != MVT::i64)
-    return false;
-
-  return !isa<ConstantSDNode>(Y) || cast<ConstantSDNode>(Y)->isOpaque();
+  // Can use andn for any scalar integer.
+  return VT.isScalarInteger();
 }
 
 bool X86TargetLowering::hasAndNot(SDValue Y) const {
@@ -3511,7 +3505,6 @@ bool X86TargetLowering::hasAndNot(SDValue Y) const {
     return hasAndNotCompare(Y);
 
   // Vector.
-
   if (!Subtarget.hasSSE1() || VT.getSizeInBits() < 128)
     return false;
 
@@ -56333,42 +56326,6 @@ static SDValue combineSetCC(SDNode *N, SelectionDAG &DAG,
 
   if (CC == ISD::SETNE || CC == ISD::SETEQ) {
     if (OpVT.isScalarInteger()) {
-      // cmpeq(or(X,Y),X) --> cmpeq(and(~X,Y),0)
-      // cmpne(or(X,Y),X) --> cmpne(and(~X,Y),0)
-      auto MatchOrCmpEq = [&](SDValue N0, SDValue N1) {
-        if (N0.getOpcode() == ISD::OR && N0->hasOneUse()) {
-          if (N0.getOperand(0) == N1)
-            return DAG.getNode(ISD::AND, DL, OpVT, DAG.getNOT(DL, N1, OpVT),
-                               N0.getOperand(1));
-          if (N0.getOperand(1) == N1)
-            return DAG.getNode(ISD::AND, DL, OpVT, DAG.getNOT(DL, N1, OpVT),
-                               N0.getOperand(0));
-        }
-        return SDValue();
-      };
-      if (SDValue AndN = MatchOrCmpEq(LHS, RHS))
-        return DAG.getSetCC(DL, VT, AndN, DAG.getConstant(0, DL, OpVT), CC);
-      if (SDValue AndN = MatchOrCmpEq(RHS, LHS))
-        return DAG.getSetCC(DL, VT, AndN, DAG.getConstant(0, DL, OpVT), CC);
-
-      // cmpeq(and(X,Y),Y) --> cmpeq(and(~X,Y),0)
-      // cmpne(and(X,Y),Y) --> cmpne(and(~X,Y),0)
-      auto MatchAndCmpEq = [&](SDValue N0, SDValue N1) {
-        if (N0.getOpcode() == ISD::AND && N0->hasOneUse()) {
-          if (N0.getOperand(0) == N1)
-            return DAG.getNode(ISD::AND, DL, OpVT, N1,
-                               DAG.getNOT(DL, N0.getOperand(1), OpVT));
-          if (N0.getOperand(1) == N1)
-            return DAG.getNode(ISD::AND, DL, OpVT, N1,
-                               DAG.getNOT(DL, N0.getOperand(0), OpVT));
-        }
-        return SDValue();
-      };
-      if (SDValue AndN = MatchAndCmpEq(LHS, RHS))
-        return DAG.getSetCC(DL, VT, AndN, DAG.getConstant(0, DL, OpVT), CC);
-      if (SDValue AndN = MatchAndCmpEq(RHS, LHS))
-        return DAG.getSetCC(DL, VT, AndN, DAG.getConstant(0, DL, OpVT), CC);
-
       // cmpeq(trunc(x),C) --> cmpeq(x,C)
       // cmpne(trunc(x),C) --> cmpne(x,C)
       // iff x upper bits are zero.

@AZero13 AZero13 requested a review from RKSimon September 9, 2025 15:36
@AZero13 AZero13 changed the title [X86] Remove redundant code in X86 [X86] Remove redundant and-not pattern code in X86 Sep 9, 2025
@AZero13
Copy link
Contributor Author

AZero13 commented Sep 9, 2025

@topperc

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

I still don't understand why you're messing with hasAndNot / hasAndNotCompare

@AZero13 AZero13 requested a review from RKSimon September 10, 2025 14:04
@AZero13
Copy link
Contributor Author

AZero13 commented Sep 10, 2025

I still don't understand why you're messing with hasAndNot / hasAndNotCompare

They will not trigger the folds otherwise. The folds that were hardcoded in X86 because they were not happening in general.

These transforms are now handled in DAGCombine, so enable hasAndNotCompare for the same cases for X86, and remove the platform-specific code that does the same thing.
Copy link

github-actions bot commented Sep 14, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

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.

4 participants