[AMDGPU] Change numBitsSigned for simplicity and document it. NFC.

[foad]

Summary

Change numBitsSigned to return the minimum size of a signed integer that
can hold the value. This is different by one from the previous result
but is more consistent with numBitsUnsigned. Update all callers. All
callers are now more consistent between the signed and unsigned cases,
and some callers get simpler, especially the ones that deal with
quantities like numBitsSigned(LHS) + numBitsSigned(RHS).

llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp

class AMDGPUCodeGenPrepare : public FunctionPass,
   /// bits, replacing \p I with 32 bit 'bitreverse' intrinsic, shifting the
   /// result of 32 bit 'bitreverse' intrinsic to the right with zero fill (the
   /// shift amount is 32 minus \p I's base element bit width), and truncating
   /// the result of the shift operation back to \p I's original type.
   ///
   /// \returns True.
   bool promoteUniformBitreverseToI32(IntrinsicInst &I) const;
 
-
+  /// \returns The minimum number of bits needed to store the value of \Op as an
+  /// unsigned integer. Truncating to this size and then zero-extending to
+  /// ScalarSize will not change the value.
   unsigned numBitsUnsigned(Value *Op, unsigned ScalarSize) const;
+
+  /// \returns The minimum number of bits needed to store the value of \Op as a
+  /// signed integer. Truncating to this size and then sign-extending to
+  /// ScalarSize will not change the value.
   unsigned numBitsSigned(Value *Op, unsigned ScalarSize) const;
 
   /// Replace mul instructions with llvm.amdgcn.mul.u24 or llvm.amdgcn.mul.s24.
   /// SelectionDAG has an issue where an and asserting the bits are known
   bool replaceMulWithMul24(BinaryOperator &I) const;
 
   /// Perform same function as equivalently named function in DAGCombiner. Since
   /// we expand some divisions here, we need to perform this before obscuring.
unsigned AMDGPUCodeGenPrepare::numBitsUnsigned(Value *Op,
   KnownBits Known = computeKnownBits(Op, *DL, 0, AC);
   return ScalarSize - Known.countMinLeadingZeros();
 }
 
 unsigned AMDGPUCodeGenPrepare::numBitsSigned(Value *Op,
                                              unsigned ScalarSize) const {
   // In order for this to be a signed 24-bit value, bit 23, must
   // be a sign bit.
-  return ScalarSize - ComputeNumSignBits(Op, *DL, 0, AC);
+  return ScalarSize - ComputeNumSignBits(Op, *DL, 0, AC) + 1;
 }
 
 static void extractValues(IRBuilder<> &Builder,
                           SmallVectorImpl<Value *> &Values, Value *V) {
   auto *VT = dyn_cast<FixedVectorType>(V->getType());
   if (!VT) {
     Values.push_back(V);
     return;
static Value *insertValues(IRBuilder<> &Builder,
   return NewVal;
 }
 
 // Returns 24-bit or 48-bit (as per `NumBits` and `Size`) mul of `LHS` and
 // `RHS`. `NumBits` is the number of KnownBits of the result and `Size` is the
 // width of the original destination.
 static Value *getMul24(IRBuilder<> &Builder, Value *LHS, Value *RHS,
                        unsigned Size, unsigned NumBits, bool IsSigned) {
-  if (Size <= 32 || (IsSigned ? NumBits <= 30 : NumBits <= 32)) {
+  if (Size <= 32 || NumBits <= 32) {
     Intrinsic::ID ID =
         IsSigned ? Intrinsic::amdgcn_mul_i24 : Intrinsic::amdgcn_mul_u24;
     return Builder.CreateIntrinsic(ID, {}, {LHS, RHS});
   }
 
-  assert(IsSigned ? NumBits <= 46 : NumBits <= 48);
+  assert(NumBits <= 48);
 
   Intrinsic::ID LoID =
       IsSigned ? Intrinsic::amdgcn_mul_i24 : Intrinsic::amdgcn_mul_u24;
   Intrinsic::ID HiID =
       IsSigned ? Intrinsic::amdgcn_mulhi_i24 : Intrinsic::amdgcn_mulhi_u24;
 
   Value *Lo = Builder.CreateIntrinsic(LoID, {}, {LHS, RHS});
   Value *Hi = Builder.CreateIntrinsic(HiID, {}, {LHS, RHS});
bool AMDGPUCodeGenPrepare::replaceMulWithMul24(BinaryOperator &I) const {
 
   unsigned LHSBits = 0, RHSBits = 0;
   bool IsSigned = false;
 
   if (ST->hasMulU24() && (LHSBits = numBitsUnsigned(LHS, Size)) <= 24 &&
       (RHSBits = numBitsUnsigned(RHS, Size)) <= 24) {
     IsSigned = false;
 
-  } else if (ST->hasMulI24() &&
-             (LHSBits = numBitsSigned(LHS, Size)) < 24 &&
-             (RHSBits = numBitsSigned(RHS, Size)) < 24) {
+  } else if (ST->hasMulI24() && (LHSBits = numBitsSigned(LHS, Size)) <= 24 &&
+             (RHSBits = numBitsSigned(RHS, Size)) <= 24) {
     IsSigned = true;
 
   } else
     return false;
 
   SmallVector<Value *, 4> LHSVals;
   SmallVector<Value *, 4> RHSVals;
   SmallVector<Value *, 4> ResultVals;

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h

private:
 
   /// \returns AMDGPUISD::FFBH_U32 node if the incoming \p Op may have been
   /// legalized from a smaller type VT. Need to match pre-legalized type because
   /// the generic legalization inserts the add/sub between the select and
   /// compare.
   SDValue getFFBX_U32(SelectionDAG &DAG, SDValue Op, const SDLoc &DL, unsigned Opc) const;
 
 public:
+  /// \returns The minimum number of bits needed to store the value of \Op as an
+  /// unsigned integer. Truncating to this size and then zero-extending to the
+  /// original size will not change the value.
   static unsigned numBitsUnsigned(SDValue Op, SelectionDAG &DAG);
+
+  /// \returns The minimum number of bits needed to store the value of \Op as a
+  /// signed integer. Truncating to this size and then sign-extending to the
+  /// original size will not change the value.
   static unsigned numBitsSigned(SDValue Op, SelectionDAG &DAG);
 
 protected:
   SDValue LowerEXTRACT_SUBVECTOR(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerCONCAT_VECTORS(SDValue Op, SelectionDAG &DAG) const;
   /// Split a vector store into multiple scalar stores.
   /// \returns The resulting chain.
 

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp

unsigned AMDGPUTargetLowering::numBitsUnsigned(SDValue Op, SelectionDAG &DAG) {
   return DAG.computeKnownBits(Op).countMaxActiveBits();
 }
 
 unsigned AMDGPUTargetLowering::numBitsSigned(SDValue Op, SelectionDAG &DAG) {
   EVT VT = Op.getValueType();
 
   // In order for this to be a signed 24-bit value, bit 23, must
   // be a sign bit.
-  return VT.getSizeInBits() - DAG.ComputeNumSignBits(Op);
+  return VT.getSizeInBits() - DAG.ComputeNumSignBits(Op) + 1;
 }
 
 AMDGPUTargetLowering::AMDGPUTargetLowering(const TargetMachine &TM,
                                            const AMDGPUSubtarget &STI)
     : TargetLowering(TM), Subtarget(&STI) {
   // Lower floating point store/load to integer store/load to reduce the number
   // of patterns in tablegen.
   setOperationAction(ISD::LOAD, MVT::f32, Promote);

 static bool isU24(SDValue Op, SelectionDAG &DAG) {
   return AMDGPUTargetLowering::numBitsUnsigned(Op, DAG) <= 24;
 }
 
 static bool isI24(SDValue Op, SelectionDAG &DAG) {
   EVT VT = Op.getValueType();
   return VT.getSizeInBits() >= 24 && // Types less than 24-bit should be treated
                                      // as unsigned 24-bit values.
-    AMDGPUTargetLowering::numBitsSigned(Op, DAG) < 24;
+         AMDGPUTargetLowering::numBitsSigned(Op, DAG) <= 24;
 }
 
 static SDValue simplifyMul24(SDNode *Node24,
                              TargetLowering::DAGCombinerInfo &DCI) {
   SelectionDAG &DAG = DCI.DAG;
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
   bool IsIntrin = Node24->getOpcode() == ISD::INTRINSIC_WO_CHAIN;
 

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

if ((LHS.getOpcode() == ISD::MUL || RHS.getOpcode() == ISD::MUL)
     if (numBitsUnsigned(MulLHS, DAG) <= 32 &&
         numBitsUnsigned(MulRHS, DAG) <= 32) {
       MulLHS = DAG.getZExtOrTrunc(MulLHS, SL, MVT::i32);
       MulRHS = DAG.getZExtOrTrunc(MulRHS, SL, MVT::i32);
       AddRHS = DAG.getZExtOrTrunc(AddRHS, SL, MVT::i64);
       return getMad64_32(DAG, SL, VT, MulLHS, MulRHS, AddRHS, false);
     }
 
-    if (numBitsSigned(MulLHS, DAG) < 32 && numBitsSigned(MulRHS, DAG) < 32) {
+    if (numBitsSigned(MulLHS, DAG) <= 32 && numBitsSigned(MulRHS, DAG) <= 32) {
       MulLHS = DAG.getSExtOrTrunc(MulLHS, SL, MVT::i32);
       MulRHS = DAG.getSExtOrTrunc(MulRHS, SL, MVT::i32);
       AddRHS = DAG.getSExtOrTrunc(AddRHS, SL, MVT::i64);
       return getMad64_32(DAG, SL, VT, MulLHS, MulRHS, AddRHS, true);
     }
 
     return SDValue();
   }