[SelectionDAG]: Deduce KnownNeverZero from SMIN and SMAX

[AZero13]

No description provided.

[llvmbot]

@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);

[jayfoad]

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]

Code LGTM but I haven't been involved too much in the review so I'll leave it for someone else to approve.

[RKSimon]

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

[AZero13]

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

[RKSimon]

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

[AZero13]

@RKSimon Thank you!

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[github-actions]

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

[RKSimon]

Please can you rebase - I've added better target coverage to known-never-zero.ll

[AZero13]

@RKSimon Done!

[AZero13]

@RKSimon Done!