[SVE] Make EVT::getScalarSizeInBits and others consistent with Type::getScalarSizeInBits

[david-arm]

Summary

An existing function Type::getScalarSizeInBits returns a uint64_t
instead of a TypeSize class because the caller is requesting a
scalar size, which cannot be scalable. This patch makes other
similar functions requesting a scalar size consistent with that,
thereby eliminating more than 800 implicit TypeSize -> uint64_t
casts.

[efriedma]

I think this makes sense, but I'm interested to see if anyone else has a different opinion.

thereby eliminating more than 1000 implicit TypeSize -> uint64_t casts.

I'm curious, how many are there in total?

[david-arm]

Hi @efriedma, so it turns out it's over 800, not 1000. :) My initial guess was a bit too high, but here are the counts I found:

grep -R getScalarValueSizeInBits llvm/ | wc -l
149

grep -R getScalarSizeInBits llvm/lib/CodeGen/SelectionDAG/ | wc -l
214

grep -R getScalarSizeInBits llvm/lib/Target/ | wc -l
456

Of course, a small percentage of calls to getScalarSizeInBits could be to the existing IR Type class member function, but having looked through the results I think are most are for MVTs and EVTs.

[sdesmalen]

LGTM. I think this is a sensible change. TypeSize is used to indicate that a value could be scalable, but if it is known the type is always fixed-size this change simplifies code that calls getScalarSizeInBits (i.e. no need to call 'getFixedSize`).
It also puts us on the path towards removing the implicit casts to uint64_t.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

7443–7444

s/OrigEltTy.getSizeInBits().getFixedSize()/OrigEltTy.getScalarSizeInBits()/ ?

llvm/lib/Target/ARM/ARMISelLowering.cpp

7848

s/OrigEltTy.getSizeInBits().getFixedSize()/OrigEltTy.getScalarSizeInBits()/ ?

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

public:
   ///
   /// If the value type is a scalable vector type, the scalable property will
   /// be set and the runtime size will be a positive integer multiple of the
   /// base size.
   TypeSize getValueSizeInBits() const {
     return getValueType().getSizeInBits();
   }
 
-  TypeSize getScalarValueSizeInBits() const {
-    return getValueType().getScalarType().getSizeInBits();
+  uint64_t getScalarValueSizeInBits() const {
+    return getValueType().getScalarType().getSizeInBits().getFixedSize();
   }
 
   // Forwarding methods - These forward to the corresponding methods in SDNode.
   inline unsigned getOpcode() const;
   inline unsigned getNumOperands() const;
   inline const SDValue &getOperand(unsigned i) const;
   inline uint64_t getConstantOperandVal(unsigned i) const;
   inline const APInt &getConstantOperandAPInt(unsigned i) const;

llvm/include/llvm/CodeGen/ValueTypes.h

#endif
     /// be set and the runtime size will be a positive integer multiple of the
     /// base size.
     TypeSize getSizeInBits() const {
       if (isSimple())
         return V.getSizeInBits();
       return getExtendedSizeInBits();
     }
 
-    TypeSize getScalarSizeInBits() const {
-      return getScalarType().getSizeInBits();
+    uint64_t getScalarSizeInBits() const {
+      return getScalarType().getSizeInBits().getFixedSize();
     }
 
     /// Return the number of bytes overwritten by a store of the specified value
     /// type.
     ///
     /// If the value type is a scalable vector type, the scalable property will
     /// be set and the runtime size will be a positive integer multiple of the
     /// base size.

llvm/include/llvm/Support/MachineValueType.h

TypeSize getSizeInBits() const {
       case v1024i32:
       case v1024f32:  return TypeSize::Fixed(32768);
       case v2048i32:
       case v2048f32:  return TypeSize::Fixed(65536);
       case exnref: return TypeSize::Fixed(0); // opaque type
       }
     }
 
-    TypeSize getScalarSizeInBits() const {
-      return getScalarType().getSizeInBits();
+    uint64_t getScalarSizeInBits() const {
+      return getScalarType().getSizeInBits().getFixedSize();
     }
 
     /// Return the number of bytes overwritten by a store of the specified value
     /// type.
     ///
     /// If the value type is a scalable vector type, the scalable property will
     /// be set and the runtime size will be a positive integer multiple of the
     /// base size.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (MLoad && BVec && MLoad->getExtensionType() == ISD::EXTLOAD &&
         N0.hasOneUse() && N1.hasOneUse()) {
       EVT LoadVT = MLoad->getMemoryVT();
       EVT ExtVT = VT;
       if (TLI.isLoadExtLegal(ISD::ZEXTLOAD, ExtVT, LoadVT)) {
         // For this AND to be a zero extension of the masked load the elements
         // of the BuildVec must mask the bottom bits of the extended element
         // type
         if (ConstantSDNode *Splat = BVec->getConstantSplatNode()) {
-          TypeSize ElementSize =
+          uint64_t ElementSize =
               LoadVT.getVectorElementType().getScalarSizeInBits();
-          if (Splat->getAPIntValue().isMask((uint64_t)ElementSize)) {
+          if (Splat->getAPIntValue().isMask(ElementSize)) {
             return DAG.getMaskedLoad(
                 ExtVT, SDLoc(N), MLoad->getChain(), MLoad->getBasePtr(),
                 MLoad->getOffset(), MLoad->getMask(), MLoad->getPassThru(),
                 LoadVT, MLoad->getMemOperand(), MLoad->getAddressingMode(),
                 ISD::ZEXTLOAD, MLoad->isExpandingLoad());
           }
         }
       }

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

case ISD::ROTR:
     assert(VT.isInteger() && N2.getValueType().isInteger() &&
            "Shifts only work on integers");
     assert((!VT.isVector() || VT == N2.getValueType()) &&
            "Vector shift amounts must be in the same as their first arg");
     // Verify that the shift amount VT is big enough to hold valid shift
     // amounts.  This catches things like trying to shift an i1024 value by an
     // i8, which is easy to fall into in generic code that uses
     // TLI.getShiftAmount().
-    assert(N2.getValueType().getScalarSizeInBits().getFixedSize() >=
-               Log2_32_Ceil(VT.getScalarSizeInBits().getFixedSize()) &&
+    assert(N2.getValueType().getScalarSizeInBits() >=
+               Log2_32_Ceil(VT.getScalarSizeInBits()) &&
            "Invalid use of small shift amount with oversized value!");
 
     // Always fold shifts of i1 values so the code generator doesn't need to
     // handle them.  Since we know the size of the shift has to be less than the
     // size of the value, the shift/rotate count is guaranteed to be zero.
     if (VT == MVT::i1)
       return N1;
     if (N2C && N2C->isNullValue())

llvm/lib/CodeGen/TargetLoweringBase.cpp

static unsigned getVectorTypeBreakdownMVT(MVT VT, MVT &IntermediateVT,
 
   NumIntermediates = NumVectorRegs;
 
   MVT NewVT = MVT::getVectorVT(EltTy, EC);
   if (!TLI->isTypeLegal(NewVT))
     NewVT = EltTy;
   IntermediateVT = NewVT;
 
-  unsigned LaneSizeInBits = NewVT.getScalarSizeInBits().getFixedSize();
+  unsigned LaneSizeInBits = NewVT.getScalarSizeInBits();
 
   // Convert sizes such as i33 to i64.
   if (!isPowerOf2_32(LaneSizeInBits))
     LaneSizeInBits = NextPowerOf2(LaneSizeInBits);
 
   MVT DestVT = TLI->getRegisterType(NewVT);
   RegisterVT = DestVT;
   if (EVT(DestVT).bitsLT(NewVT))    // Value is expanded, e.g. i64 -> i16.
-    return NumVectorRegs *
-           (LaneSizeInBits / DestVT.getScalarSizeInBits().getFixedSize());
+    return NumVectorRegs * (LaneSizeInBits / DestVT.getScalarSizeInBits());
 
   // Otherwise, promotion or legal types use the same number of registers as
   // the vector decimated to the appropriate level.
   return NumVectorRegs;
 }
 
 /// isLegalRC - Return true if the value types that can be represented by the
 /// specified register class are all legal.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

for (unsigned i = 0; i < VT.getVectorNumElements(); ++i) {
 
     auto Src = find(Sources, Entry.getOperand(0));
     int EltNo = cast<ConstantSDNode>(Entry.getOperand(1))->getSExtValue();
 
     // EXTRACT_VECTOR_ELT performs an implicit any_ext; BUILD_VECTOR an implicit
     // trunc. So only std::min(SrcBits, DestBits) actually get defined in this
     // segment.
     EVT OrigEltTy = Entry.getOperand(0).getValueType().getVectorElementType();
-    int BitsDefined =
-        std::min(OrigEltTy.getSizeInBits(), VT.getScalarSizeInBits());
+    int BitsDefined = std::min(OrigEltTy.getScalarSizeInBits(),
+                               VT.getScalarSizeInBits());
     int LanesDefined = BitsDefined / BitsPerShuffleLane;
 
     // This source is expected to fill ResMultiplier lanes of the final shuffle,
     // starting at the appropriate offset.
     int *LaneMask = &Mask[i * ResMultiplier];
 
     int ExtractBase = EltNo * Src->WindowScale + Src->WindowBase;
     ExtractBase += NumElts * (Src - Sources.begin());

llvm/lib/Target/ARM/ARMISelLowering.cpp

for (unsigned i = 0; i < VT.getVectorNumElements(); ++i) {
 
     auto Src = llvm::find(Sources, Entry.getOperand(0));
     int EltNo = cast<ConstantSDNode>(Entry.getOperand(1))->getSExtValue();
 
     // EXTRACT_VECTOR_ELT performs an implicit any_ext; BUILD_VECTOR an implicit
     // trunc. So only std::min(SrcBits, DestBits) actually get defined in this
     // segment.
     EVT OrigEltTy = Entry.getOperand(0).getValueType().getVectorElementType();
-    int BitsDefined = std::min(OrigEltTy.getSizeInBits(),
+    int BitsDefined = std::min(OrigEltTy.getScalarSizeInBits(),
                                VT.getScalarSizeInBits());
     int LanesDefined = BitsDefined / BitsPerShuffleLane;
 
     // This source is expected to fill ResMultiplier lanes of the final shuffle,
     // starting at the appropriate offset.
     int *LaneMask = &Mask[i * ResMultiplier];
 
     int ExtractBase = EltNo * Src->WindowScale + Src->WindowBase;