[LegalizeTypes] Handle gaps in legal vector types while widening loads

[frasercrmck]

Summary

On targets where there are gaps in legal vector types then
GenWidenVectorLoads would attempt to create invalid CONCAT_VECTORS nodes
when widening certain vector loads.

For example, on a target where v4i32 and v16i32 are legal (i.e. there is
a "gap" where v8i32 should be) then legalizing v24i32 would attempt to
create node thus:

  v16i32 = CONCAT_VECTORS v4i32, v4i32

The fix is to correctly pad CONCAT_VECTORS nodes with undefs in all
cases.

[frasercrmck]

Note that the best test I've found so far is this:

define void @ldv24i32(<24 x i32>* %ptr) {
  entry:
  %ld = load volatile <24 x i32>, <24 x i32>* %ptr,  align 16
  ret void
}

... but only after editing AMDGPU/SIISelLowering.cpp to remove v8i32 as a legal type.

If anyone knows a better target to test on, I would appreciate the advice.

[frasercrmck]

Ping

[frasercrmck]

This appears to have been committed by @craig.topper in D84463. Great minds, and all that.

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

while (LdWidth > 0) {
     Offset += Increment;
     BasePtr = DAG.getObjectPtrOffset(dl, BasePtr, Increment);
 
     SDValue L;
     if (LdWidth < NewVTWidth) {
       // The current type we are using is too large. Find a better size.
       NewVT = FindMemType(DAG, TLI, LdWidth, WidenVT, LdAlign, WidthDiff);
       NewVTWidth = NewVT.getSizeInBits();
-      L = DAG.getLoad(NewVT, dl, Chain, BasePtr,
-                      LD->getPointerInfo().getWithOffset(Offset),
-                      MinAlign(Align, Increment), MMOFlags, AAInfo);
-      LdChain.push_back(L.getValue(1));
-      if (L->getValueType(0).isVector() && NewVTWidth >= LdWidth) {
-        // Later code assumes the vector loads produced will be mergeable, so we
-        // must pad the final entry up to the previous width. Scalars are
-        // combined separately.
-        SmallVector<SDValue, 16> Loads;
-        Loads.push_back(L);
-        unsigned size = L->getValueSizeInBits(0);
-        while (size < LdOp->getValueSizeInBits(0)) {
-          Loads.push_back(DAG.getUNDEF(L->getValueType(0)));
-          size += L->getValueSizeInBits(0);
-        }
-        L = DAG.getNode(ISD::CONCAT_VECTORS, dl, LdOp->getValueType(0), Loads);
-      }
-    } else {
-      L = DAG.getLoad(NewVT, dl, Chain, BasePtr,
-                      LD->getPointerInfo().getWithOffset(Offset),
-                      MinAlign(Align, Increment), MMOFlags, AAInfo);
-      LdChain.push_back(L.getValue(1));
-    }
+
 
     LdOps.push_back(L);
     LdOp = L;
 
     LdWidth -= NewVTWidth;
   }
 
   // Build the vector from the load operations.
for (--i; i >= 0; --i) {
         break;
     }
     ConcatOps[--Idx] = BuildVectorFromScalar(DAG, LdTy, LdOps, i + 1, End);
   }
   ConcatOps[--Idx] = LdOps[i];
   for (--i; i >= 0; --i) {
     EVT NewLdTy = LdOps[i].getValueType();
     if (NewLdTy != LdTy) {
+      // We may need to fill the rest with undefs to build the larger vector.
+      unsigned NumOps = NewLdTy.getSizeInBits() / LdTy.getSizeInBits();
+      SmallVector<SDValue, 16> WidenOps(NumOps);
+      SDValue UndefVal = DAG.getUNDEF(LdTy);
+      {
+        unsigned i = 0;
+        for (; i != End - Idx; ++i)
+          WidenOps[i] = ConcatOps[Idx + i];
+        for (; i != NumOps; ++i)
+          WidenOps[i] = UndefVal;
+      }
       // Create a larger vector.
-      ConcatOps[End-1] = DAG.getNode(ISD::CONCAT_VECTORS, dl, NewLdTy,
-                                     makeArrayRef(&ConcatOps[Idx], End - Idx));
+      ConcatOps[End - 1] =
+          DAG.getNode(ISD::CONCAT_VECTORS, dl, NewLdTy, WidenOps);
       Idx = End - 1;
       LdTy = NewLdTy;
     }
     ConcatOps[--Idx] = LdOps[i];
   }
 
   if (WidenWidth == LdTy.getSizeInBits() * (End - Idx))
     return DAG.getNode(ISD::CONCAT_VECTORS, dl, WidenVT,