llvm-project icon indicating copy to clipboard operation
llvm-project copied to clipboard

[SelectionDAG]: Deduce KnownNeverZero from SMIN and SMAX

Open AZero13 opened this issue 1 year ago • 7 comments

AZero13 avatar Mar 19 '24 00:03 AZero13

@llvm/pr-subscribers-llvm-regalloc @llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Author: AtariDreams (AtariDreams)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+20-3)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 6f6ed4bd45027b..81d2df1db565ea 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -4007,8 +4007,6 @@ KnownBits SelectionDAG::computeKnownBits(SDValue Op, const APInt &DemandedElts,
 
     // For SMAX, if CstLow is non-negative we know the result will be
     // non-negative and thus all sign bits are 0.
-    // TODO: There's an equivalent of this for smin with negative constant for
-    // known ones.
     if (IsMax && CstLow) {
       const APInt &ValueLow = CstLow->getAPIntValue();
       if (ValueLow.isNonNegative()) {
@@ -4017,6 +4015,16 @@ KnownBits SelectionDAG::computeKnownBits(SDValue Op, const APInt &DemandedElts,
       }
     }
 
+    // For SMAX, if CstHigh is negative we know the result will be
+    // negative and thus all sign bits are 1.
+    if (!IsMax && CstHigh) {
+      const APInt &ValueHigh = CstHigh->getAPIntValue();
+      if (ValueHigh.isNegative()) {
+        unsigned SignBits = ComputeNumSignBits(Op.getOperand(0), Depth + 1);
+        Known.One.setHighBits(std::min(SignBits, ValueHigh.getNumSignBits()));
+      }
+    }
+
     break;
   }
   case ISD::UINT_TO_FP: {
@@ -5360,10 +5368,19 @@ bool SelectionDAG::isKnownNeverZero(SDValue Op, unsigned Depth) const {
     return isKnownNeverZero(Op.getOperand(1), Depth + 1) ||
            isKnownNeverZero(Op.getOperand(0), Depth + 1);
 
-    // TODO for smin/smax: If either operand is known negative/positive
+    // For smin/smax: If either operand is known negative/positive
     // respectively we don't need the other to be known at all.
   case ISD::SMAX:
+    if (computeKnownBits(Op.getOperand(0), Depth + 1).isStrictlyPositive() ||
+        computeKnownBits(Op.getOperand(1), Depth + 1).isStrictlyPositive())
+      return true;
+    return isKnownNeverZero(Op.getOperand(1), Depth + 1) &&
+           isKnownNeverZero(Op.getOperand(0), Depth + 1);
   case ISD::SMIN:
+    if (computeKnownBits(Op.getOperand(0), Depth + 1).isNegative() ||
+        computeKnownBits(Op.getOperand(1), Depth + 1).isNegative())
+      return isKnownNeverZero(Op.getOperand(1), Depth + 1) &&
+             isKnownNeverZero(Op.getOperand(0), Depth + 1);
   case ISD::UMIN:
     return isKnownNeverZero(Op.getOperand(1), Depth + 1) &&
            isKnownNeverZero(Op.getOperand(0), Depth + 1);

llvmbot avatar Mar 19 '24 00:03 llvmbot

Maybe split this into two patches, one for computeKnownBits and one for isKnownNeverZero? Then it is easier to see what is being tested where.

jayfoad avatar Mar 19 '24 06:03 jayfoad

You need better vector tests - the vector CTTZ expansion won't be making use of is never known zero

So, what would then, if I may ask? Just to give me an idea so I could make a test for it

AZero13 avatar Mar 22 '24 14:03 AZero13

simplifySetCCWithCTPOP appears to use isKnownNeverZero for the (ctpop x) eq/ne 1 --> (x & x-1) eq/ne 0 fold - maybe try that?

RKSimon avatar Mar 22 '24 15:03 RKSimon

@RKSimon Thank you!

AZero13 avatar Mar 22 '24 21:03 AZero13

:white_check_mark: With the latest revision this PR passed the Python code formatter.

github-actions[bot] avatar Mar 22 '24 21:03 github-actions[bot]

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

github-actions[bot] avatar Mar 22 '24 21:03 github-actions[bot]

@RKSimon Done!

AZero13 avatar Mar 24 '24 16:03 AZero13

@RKSimon Done!

AZero13 avatar Mar 24 '24 18:03 AZero13