[SelectionDAG] TargetLowering::SimplifyDemandedBits how to properly calculate KnownZero bits for ISD::SETCC and ISD::AssertZExt

[craig.topper]

Summary

For SETCC we aren't calculating the KnownZero bits at all. I've copied the code from computeKnownZero over for this.

For AssertZExt we were only setting KnownZero for bits that were demanded. But the upper bits are zero whether they were demanded or not.

I'm interested in fixing this because my belief is the first part of the ISD::AND handling code in SimplifyDemandedBits largely exists because of these two bugs. In that code we go to computeKnownBits for the LHS and optimize a RHS constant. Because computeKnownBits handles SETCC and AssertZExt correctly we get better information sometimes than when we call SimplifyDemandedBits on the LHS later. With these two issues fixed in SimplifyDemandedBits I was able to remove that computeKnownBits call and still pass all X86 tests. I'll submit that change in a separate patch.

[RKSimon]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

794 ↗

(On Diff #94253)

Use Op0 directly?

[craig.topper]

Addressed Simon's comment.

[RKSimon]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

794 ↗

(On Diff #94253)

Come to think of it - why are you testing the input op type and not the result type (Op.getValueType()) like above?

[craig.topper]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

794 ↗

(On Diff #94253)

I sort of wondered that too, but I just copied the code out of computeKnownBits

[RKSimon]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

794 ↗

(On Diff #94253)

I think this needs fixing first - I made the change in computeKnownBits and it affected a number of MIPS tests - I'll have a look at what is going on.

[RKSimon]

LGTM

lib/CodeGen/SelectionDAG/TargetLowering.cpp

794 ↗

(On Diff #94253)

False alarm - it's doing what its supposed to and is using the input type correctly to call TLI::getBooleanContents

llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp

if (NewMask.isSignBit() && Op0.getScalarValueSizeInBits() == BitWidth &&
       // that we don't care about NaNs.
       if (CC == ISD::SETLT && Op1.getValueType().isInteger() &&
           (isNullConstant(Op1) || ISD::isBuildVectorAllZeros(Op1.getNode())))
         return TLO.CombineTo(Op, Op0);
 
       // TODO: Should we check for other forms of sign-bit comparisons?
       // Examples: X <= -1, X >= 0
     }
+    if (getBooleanContents(Op0.getValueType()) ==
+            TargetLowering::ZeroOrOneBooleanContent &&
+        BitWidth > 1)
+      KnownZero.setBitsFrom(1);
     break;
   }
   case ISD::SHL:
     if (ConstantSDNode *SA = dyn_cast<ConstantSDNode>(Op.getOperand(1))) {
       unsigned ShAmt = SA->getZExtValue();
       SDValue InOp = Op.getOperand(0);
 
       // If the shift count is an invalid immediate, don't do anything.
case ISD::AssertZext: {
     EVT VT = cast<VTSDNode>(Op.getOperand(1))->getVT();
     APInt InMask = APInt::getLowBitsSet(BitWidth,
                                         VT.getSizeInBits());
     if (SimplifyDemandedBits(Op.getOperand(0), ~InMask | NewMask,
                              KnownZero, KnownOne, TLO, Depth+1))
       return true;
     assert((KnownZero & KnownOne) == 0 && "Bits known to be one AND zero?");
 
-    KnownZero |= ~InMask & NewMask;
+    KnownZero |= ~InMask;
     break;
   }
   case ISD::BITCAST:
     // If this is an FP->Int bitcast and if the sign bit is the only
     // thing demanded, turn this into a FGETSIGN.
     if (!TLO.LegalOperations() &&
         !Op.getValueType().isVector() &&
         !Op.getOperand(0).getValueType().isVector() &&