[SVE][CodeGen] Fix TypeSize/ElementCount related warnings in sve-split-load.ll

[david-arm]

Summary

I have fixed up a number of warnings resulting from TypeSize -> uint64_t
casts and calling getVectorNumElements() on scalable vector types. I
think most of the changes are fairly trivial except for those in
DAGTypeLegalizer::SplitVecRes_MLOAD - I've tried to ensure we create
the MachineMemoryOperands in a sensible way for scalable vectors.

I have added a CHECK line to the following test:

CodeGen/AArch64/sve-split-load.ll

that ensures no new warnings are added.

[efriedma]

llvm/include/llvm/Support/TypeSize.h

229

This name is possibly misleading; the runtime size might not be a power of two even if isPowerOf2() returns true.

In the one place you're using it, the type can't be a vector type, anyway.

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

1663

I thought we added some form of pointer increment that took care of the MachinePointerInfo? Or am I misremembering?

Semantically, this looks correct.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

9646

Should we add a comparison operator to ElementCount, like the one we have for TypeSize?

[david-arm]

• Rebased on top of my ElementCount refactor patch.
• Removed isPowerOf2() member function from ElementCount.

[david-arm]

llvm/include/llvm/Support/TypeSize.h

229

Good point and I've removed this function so it's calculated explicitly in the code as before

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

1663

We do have IncrementPointer, but it sadly doesn't do the same thing. The code above increments the pointer using IncrementMemoryAddress, which attempts to deal with compressed memory based upon the mask.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

9646

Based on discussions in the parent patch (https://reviews.llvm.org/D86065) I think the consensus is to avoid the use of operators for comparisons. Perhaps we can revisit this at some point when we have some kind of agreed set of interfaces?

[efriedma]

LGTM with one minor comment.

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

767

isPowerOf2_64() so you don't have implicit truncation.

llvm/include/llvm/Support/TypeSize.h

public:
   ElementCount operator*(unsigned RHS) {
     return { Min * RHS, Scalable };
   }
   ElementCount operator/(unsigned RHS) {
     assert(Min % RHS == 0 && "Min is not a multiple of RHS.");
     return { Min / RHS, Scalable };
   }
 
+  friend ElementCount operator-(const ElementCount &LHS,
+                                const ElementCount &RHS) {
+    assert(LHS.Scalable == RHS.Scalable &&
+           "Arithmetic using mixed scalable and fixed types");
+    return {LHS.Min - RHS.Min, LHS.Scalable};
+  }
+
   bool operator==(const ElementCount& RHS) const {
     return Min == RHS.Min && Scalable == RHS.Scalable;
   }
   bool operator!=(const ElementCount& RHS) const {
     return !(*this == RHS);
   }
   bool operator==(unsigned RHS) const { return Min == RHS && !Scalable; }
   bool operator!=(unsigned RHS) const { return !(*this == RHS); }
uint64_t getKnownMinSize() const {
     return MinSize;
   }
 
   // Return whether or not the size is scalable.
   bool isScalable() const {
     return IsScalable;
   }
 
+  bool isPowerOf2() const { return (MinSize & (MinSize - 1)) == 0; }
+
   // Returns true if the number of bits is a multiple of an 8-bit byte.
   bool isByteSized() const {
     return (MinSize & 7) == 0;
   }
 
   // Returns true if the type size is non-zero.
   bool isNonZero() const { return MinSize != 0; }
 

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

if (RChain.getNode() != Node) {
       }
       ReplacedNode(Node);
     }
     return;
   }
 
   LLVM_DEBUG(dbgs() << "Legalizing extending load operation\n");
   EVT SrcVT = LD->getMemoryVT();
-  unsigned SrcWidth = SrcVT.getSizeInBits();
+  TypeSize SrcWidth = SrcVT.getSizeInBits();
   MachineMemOperand::Flags MMOFlags = LD->getMemOperand()->getFlags();
   AAMDNodes AAInfo = LD->getAAInfo();
 
   if (SrcWidth != SrcVT.getStoreSizeInBits() &&
       // Some targets pretend to have an i1 loading operation, and actually
       // load an i8.  This trick is correct for ZEXTLOAD because the top 7
       // bits are guaranteed to be zero; it helps the optimizers understand
       // that these bits are zero.  It is also useful for EXTLOAD, since it
if (SrcWidth != SrcVT.getStoreSizeInBits() &&
     else if (ExtType == ISD::ZEXTLOAD || NVT == Result.getValueType())
       // All the top bits are guaranteed to be zero - inform the optimizers.
       Result = DAG.getNode(ISD::AssertZext, dl,
                            Result.getValueType(), Result,
                            DAG.getValueType(SrcVT));
 
     Value = Result;
     Chain = Ch;
-  } else if (SrcWidth & (SrcWidth - 1)) {
+  } else if (!SrcWidth.isPowerOf2()) {
     // If not loading a power-of-2 number of bits, expand as two loads.
     assert(!SrcVT.isVector() && "Unsupported extload!");
-    unsigned LogSrcWidth = Log2_32(SrcWidth);
+    unsigned SrcWidthBits = SrcWidth.getFixedSize();
+    unsigned LogSrcWidth = Log2_32(SrcWidthBits);
     assert(LogSrcWidth < 32);
     unsigned RoundWidth = 1 << LogSrcWidth;
-    assert(RoundWidth < SrcWidth);
-    unsigned ExtraWidth = SrcWidth - RoundWidth;
+    assert(RoundWidth < SrcWidthBits);
+    unsigned ExtraWidth = SrcWidthBits - RoundWidth;
     assert(ExtraWidth < RoundWidth);
     assert(!(RoundWidth % 8) && !(ExtraWidth % 8) &&
            "Load size not an integral number of bytes!");
     EVT RoundVT = EVT::getIntegerVT(*DAG.getContext(), RoundWidth);
     EVT ExtraVT = EVT::getIntegerVT(*DAG.getContext(), ExtraWidth);
     SDValue Lo, Hi, Ch;
     unsigned IncrementSize;
     auto &DL = DAG.getDataLayout();

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

std::tie(LoMemVT, HiMemVT) =
       DAG.GetDependentSplitDestVTs(MemoryVT, LoVT, &HiIsEmpty);
 
   SDValue PassThruLo, PassThruHi;
   if (getTypeAction(PassThru.getValueType()) == TargetLowering::TypeSplitVector)
     GetSplitVector(PassThru, PassThruLo, PassThruHi);
   else
     std::tie(PassThruLo, PassThruHi) = DAG.SplitVector(PassThru, dl);
 
+  unsigned LoSize = MemoryLocation::getSizeOrUnknown(LoMemVT.getStoreSize());
   MachineMemOperand *MMO = DAG.getMachineFunction().getMachineMemOperand(
-      MLD->getPointerInfo(), MachineMemOperand::MOLoad, LoMemVT.getStoreSize(),
-      Alignment, MLD->getAAInfo(), MLD->getRanges());
+      MLD->getPointerInfo(), MachineMemOperand::MOLoad, LoSize, Alignment,
+      MLD->getAAInfo(), MLD->getRanges());
 
   Lo = DAG.getMaskedLoad(LoVT, dl, Ch, Ptr, Offset, MaskLo, PassThruLo, LoMemVT,
                          MMO, MLD->getAddressingMode(), ExtType,
                          MLD->isExpandingLoad());
 
   if (HiIsEmpty) {
     // The hi masked load has zero storage size. We therefore simply set it to
     // the low masked load and rely on subsequent removal from the chain.
     Hi = Lo;
   } else {
     // Generate hi masked load.
     Ptr = TLI.IncrementMemoryAddress(Ptr, MaskLo, dl, LoMemVT, DAG,
                                      MLD->isExpandingLoad());
-    unsigned HiOffset = LoMemVT.getStoreSize();
+    unsigned HiSize = MemoryLocation::getSizeOrUnknown(HiMemVT.getStoreSize());
+
+    MachinePointerInfo MPI;
+    if (LoMemVT.isScalableVector())
+      MPI = MachinePointerInfo(MLD->getPointerInfo().getAddrSpace());
+    else
+      MPI = MLD->getPointerInfo().getWithOffset(
+          LoMemVT.getStoreSize().getFixedSize());
 
     MMO = DAG.getMachineFunction().getMachineMemOperand(
-        MLD->getPointerInfo().getWithOffset(HiOffset),
-        MachineMemOperand::MOLoad, HiMemVT.getStoreSize(), Alignment,
-        MLD->getAAInfo(), MLD->getRanges());
+        MPI, MachineMemOperand::MOLoad, HiSize, Alignment, MLD->getAAInfo(),
+        MLD->getRanges());
 
     Hi = DAG.getMaskedLoad(HiVT, dl, Ch, Ptr, Offset, MaskHi, PassThruHi,
                            HiMemVT, MMO, MLD->getAddressingMode(), ExtType,
                            MLD->isExpandingLoad());
   }
 
   // Build a factor node to remember that this load is independent of the
   // other one.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

if (VT == MemVT) {
     // Extending load.
     assert(MemVT.getScalarType().bitsLT(VT.getScalarType()) &&
            "Should only be an extending load, not truncating!");
     assert(VT.isInteger() == MemVT.isInteger() &&
            "Cannot convert from FP to Int or Int -> FP!");
     assert(VT.isVector() == MemVT.isVector() &&
            "Cannot use an ext load to convert to or from a vector!");
     assert((!VT.isVector() ||
-            VT.getVectorNumElements() == MemVT.getVectorNumElements()) &&
+            VT.getVectorElementCount() == MemVT.getVectorElementCount()) &&
            "Cannot use an ext load to change the number of vector elements!");
   }
 
   bool Indexed = AM != ISD::UNINDEXED;
   assert((Indexed || Offset.isUndef()) && "Unindexed load with an offset!");
 
   SDVTList VTs = Indexed ?
     getVTList(VT, Ptr.getValueType(), MVT::Other) : getVTList(VT, MVT::Other);

 
 /// GetDependentSplitDestVTs - Compute the VTs needed for the low/hi parts of a
 /// type, dependent on an enveloping VT that has been split into two identical
 /// pieces. Sets the HiIsEmpty flag when hi type has zero storage size.
 std::pair<EVT, EVT>
 SelectionDAG::GetDependentSplitDestVTs(const EVT &VT, const EVT &EnvVT,
                                        bool *HiIsEmpty) const {
   EVT EltTp = VT.getVectorElementType();
-  bool IsScalable = VT.isScalableVector();
   // Examples:
   //   custom VL=8  with enveloping VL=8/8 yields 8/0 (hi empty)
   //   custom VL=9  with enveloping VL=8/8 yields 8/1
   //   custom VL=10 with enveloping VL=8/8 yields 8/2
   //   etc.
-  unsigned VTNumElts = VT.getVectorNumElements();
-  unsigned EnvNumElts = EnvVT.getVectorNumElements();
+  ElementCount VTNumElts = VT.getVectorElementCount();
+  ElementCount EnvNumElts = EnvVT.getVectorElementCount();
+  assert(VTNumElts.Scalable == EnvNumElts.Scalable &&
+         "Mixing fixed width and scalable vectors when enveloping a type");
   EVT LoVT, HiVT;
-  if (VTNumElts > EnvNumElts) {
+  if (VTNumElts.Min > EnvNumElts.Min) {
     LoVT = EnvVT;
-    HiVT = EVT::getVectorVT(*getContext(), EltTp, VTNumElts - EnvNumElts,
-                            IsScalable);
+    HiVT = EVT::getVectorVT(*getContext(), EltTp, VTNumElts - EnvNumElts);
     *HiIsEmpty = false;
   } else {
     // Flag that hi type has zero storage size, but return split envelop type
     // (this would be easier if vector types with zero elements were allowed).
-    LoVT = EVT::getVectorVT(*getContext(), EltTp, VTNumElts, IsScalable);
+    LoVT = EVT::getVectorVT(*getContext(), EltTp, VTNumElts);
     HiVT = EnvVT;
     *HiIsEmpty = true;
   }
   return std::make_pair(LoVT, HiVT);
 }
 
 /// SplitVector - Split the vector with EXTRACT_SUBVECTOR and return the
 /// low/high part.

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

 SDValue
 TargetLowering::IncrementMemoryAddress(SDValue Addr, SDValue Mask,
                                        const SDLoc &DL, EVT DataVT,
                                        SelectionDAG &DAG,
                                        bool IsCompressedMemory) const {
   SDValue Increment;
   EVT AddrVT = Addr.getValueType();
   EVT MaskVT = Mask.getValueType();
-  assert(DataVT.getVectorNumElements() == MaskVT.getVectorNumElements() &&
+  assert(DataVT.getVectorElementCount() == MaskVT.getVectorElementCount() &&
          "Incompatible types of Data and Mask");
   if (IsCompressedMemory) {
     if (DataVT.isScalableVector())
       report_fatal_error(
           "Cannot currently handle compressed memory with scalable vectors");
     // Incrementing the pointer according to number of '1's in the mask.
     EVT MaskIntVT = EVT::getIntegerVT(*DAG.getContext(), MaskVT.getSizeInBits());
     SDValue MaskInIntReg = DAG.getBitcast(MaskIntVT, Mask);

llvm/test/CodeGen/AArch64/sve-split-load.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; If this check fails please read test/CodeGen/AArch64/README for instructions on how to resolve it.
+; WARN-NOT: warning
 
 ; UNPREDICATED
 
 define <vscale x 4 x i16> @load_promote_4i16(<vscale x 4 x i16>* %a) {
 ; CHECK-LABEL: load_promote_4i16:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ptrue p0.s
 ; CHECK-NEXT:    ld1h { z0.s }, p0/z, [x0]