[SelectionDAG] Calculate KnownBits for SMIN

[AZero13]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

Author: AtariDreams (AtariDreams)

Changes

---

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

1 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+10-2)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 2670f48aebcff5..f6ac5d98d1d402 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 SMIN, 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: {

[topperc]

Tests? I feel like I keep asking for tests on many of your patches.

[jayfoad]

I don't understand why any of this SMIN/SMAX handling code is needed, except for the call to KnownBits::smin/smax. Those functions are optimal, and the code here does not seem to use any knowledge that is not captured by KnownBits info.

[topperc]

It's possible to know the LHS has some number of sign bits without knowing whether it is positive or negative. Here's the patch that added the SMAX code https://reviews.llvm.org/D126896

The issue was that we knew the sign bit of the smax was zero before type legalization when the type was i32. This turned a sext into a sext. Then we promoted to i64 and known bits would only say bit 63 of the i64 smax was known zero. That patch made compute known bits say at least 63:31 were known zero to match what we knew before type legalization.

[jayfoad]

Thanks - I see now that using numsignbits info does let it deduce more stuff than KnownBits can.

[arsenm]

This would be OK with tests