[RFC][DataLayout] Allow vector specifications by element size

[frasercrmck]

Summary

This patch adds the ability to configure the DataLayout specifications
on vector types by their element size, rather than their total size.

Specifying alignments by total vector size is not a good fit for some
architectures -- such as RISC-V's V extension (RVV) -- for a couple of
reasons.

The first is that for many architectures, such as those with
"Cray-style" vectors, there are a very wide set of supported vector
types, and specifying each by their total size is infeasible. RVV, for
example, could theoretically support any i8 vector type between 1 and
8192 bytes in size. Specifying optimal alignments for all such vector
types is cumbersome.

The second is that several architectures (again, like RVV) specify
vector alignment according to their element size. Thus under the current
system it is necessary either to pessimize vector alignments by rounding
them up to that of the maximum alignment across all legal vectors of
that size and/or to handle them as "misaligned" accesses.

Instead, this patch aims to both better suit and simplify the
specifications for such architectures. The ability to specify vector
alignments by element size is provided through a new specifier: ve. To
avoid any ambiguity about how this interacts with the existing v
vector specifier, the two are *mutually exclusive*; it is an error to
manually specify both in a single data layout. Since there are two v
specifiers in the set of defaults, an explicitly-set ve specifier must
be careful to erase those defaults. This decision also helps to preserve
backwards compatibility with older data layouts.

One outstanding design issue concerns how this should play with the
x86_mmx type, as that (AFAIK) doesn't have an element type.

[frasercrmck]

• adjust comment

[nikic]

Just to make sure that we don't end up with another incomplete solution: Are there any architectures where alignment could depend on both element size and total size? Are there any where it could depend on element type, i.e. where a vector of i32 and a vector of f32 may have different alignment?

llvm/lib/IR/DataLayout.cpp

401

You can use erase_if to simplify this pattern.

[frasercrmck]

In D112531#3163628, @nikic wrote:

Just to make sure that we don't end up with another incomplete solution: Are there any architectures where alignment could depend on both element size and total size? Are there any where it could depend on element type, i.e. where a vector of i32 and a vector of f32 may have different alignment?

Great question! I'm not aware of such architectures but that's not to say they don't exist. I'm not sure how best to answer that question, given all the weird and wonderful architectures out there, unless a poll on llvm-dev suffices?

I feel like a spec that can juggle both total size and element size independently in the same DataLayout string is infeasible. Neither choice of how they'd resolve themselves when in conflict is satisfying to me. If v takes precedence over ve then ve is essentially useless (because v64 and v128 are legacy-specified by default). But if ve takes precedence over v then "v-based" architectures become way more verbose. That's why I went with the mutually-exclusive design here.

But narrowing in on your word "and", then maybe if you specified them together you can view total size as a specialization which applies on top of element size.

So the first idea that's coming to me right now would be to design a specification that would encode both total size and element size/type. Something like VX?[eif]N

• (optional) X is the total size (or min size as that's how scalable vectors operate). Defaults to "all vectors". X if specified specializes over all non-specified vector sizes.
• e is agnostic element size, i is int, f is floating-point. i or f specialize over e
• N is element size

Something of a superset of this patch. It would allow something like Ve16 as this patch introduces: all 16-bit-element vectors. Vf16 could be used on top of that to apply only to all half vectors. V32f16 would apply on top of that to apply to 32-bit (2-element) half vectors.

But where does V64e16 fit into that schema? I think it would probably snuggle in between Vf16 and V32f16 but some may argue that f is more specific than e.

An alternative to total size there would be (min) number of elements, in case that was preferable. And if we decide the size/elts thing is awkward we could still keep the eif idea to allow specialization over different element types.

I think V and v would still continue to be mutually exclusive for the reasons given earlier.

I'm curious to hear your thoughts on this matter.

[nikic]

In D112531#3168987, @frasercrmck wrote:

In D112531#3163628, @nikic wrote:

Just to make sure that we don't end up with another incomplete solution: Are there any architectures where alignment could depend on both element size and total size? Are there any where it could depend on element type, i.e. where a vector of i32 and a vector of f32 may have different alignment?

Great question! I'm not aware of such architectures but that's not to say they don't exist. I'm not sure how best to answer that question, given all the weird and wonderful architectures out there, unless a poll on llvm-dev suffices?

Based on you the discussion in your comment, I think the important bit is that there is a viable future extension path: Even if we have just ve now, we can later extend that to also include a minimal total vector size etc using the scheme you suggest, if it becomes necessary. That looks like a backwards-compatible extension to me. Though probably checking with llvm-dev wouldn't hurt.

[craig.topper]

While the spec doesn't require RVV vectors to be more aligned than element size. A given microarchitecture may be more optimal if data is aligned to a VLEN chunk or perhaps a fraction of a VLEN chunk. So the preferred alignment for such a CPU would be more than element alignment. Though maybe data layout isn't the right place to handle microarchitectural differences like that.

[frasercrmck]

In D112531#3170485, @nikic wrote:

Based on you the discussion in your comment, I think the important bit is that there is a viable future extension path: Even if we have just ve now, we can later extend that to also include a minimal total vector size etc using the scheme you suggest, if it becomes necessary. That looks like a backwards-compatible extension to me. Though probably checking with llvm-dev wouldn't hurt.

I've sent out a message to llvm-dev to gather any suggestions. I'm not sure how long to leave it though.

In D112531#3170999, @craig.topper wrote:

While the spec doesn't require RVV vectors to be more aligned than element size. A given microarchitecture may be more optimal if data is aligned to a VLEN chunk or perhaps a fraction of a VLEN chunk. So the preferred alignment for such a CPU would be more than element alignment. Though maybe data layout isn't the right place to handle microarchitectural differences like that.

That's an interesting point. I agree it's not really something for the data layout but I also can't think of where better fits that sort of thing.

[frasercrmck]

• rebase

[frasercrmck]

rebase patch

[efriedma]

llvm/lib/IR/DataLayout.cpp

833

X86_MMXTyID is specifically for x86, so it's not likely to come up in practice... but should be fine to compute the alignment as if the element type is i64.

llvm/docs/LangRef.rst

``p[n]:<size>:<abi>[:<pref>][:<idx>]``
     in the range [1,2^23).
 ``i<size>:<abi>[:<pref>]``
     This specifies the alignment for an integer type of a given bit
     ``<size>``. The value of ``<size>`` must be in the range [1,2^23).
     ``<pref>`` is optional and defaults to ``<abi>``.
 ``v<size>:<abi>[:<pref>]``
     This specifies the alignment for a vector type of a given bit
     ``<size>``. The value of ``<size>`` must be in the range [1,2^23).
-    ``<pref>`` is optional and defaults to ``<abi>``.
+    ``<pref>`` is optional and defaults to ``<abi>``. This cannot be used in
+    conjunction with ``ve<size>``, below.
+``ve<size>:<abi>[:<pref>]``
+    This specifies the alignment for vector types with an element type of a
+    given bit ``<size>``. The value of ``<size>`` must be in the range
+    [1,2^23). ``<pref>`` is optional and defaults to ``<abi>``. This cannot be
+    used in conjunction with ``v<size>``, above.
 ``f<size>:<abi>[:<pref>]``
     This specifies the alignment for a floating-point type of a given bit
     ``<size>``. Only values of ``<size>`` that are supported by the target
     will work. 32 (float) and 64 (double) are supported on all targets; 80
     or 128 (different flavors of long double) are also supported on some
     targets. The value of ``<size>`` must be in the range [1,2^23).
     ``<pref>`` is optional and defaults to ``<abi>``.
 ``a:<abi>[:<pref>]``

llvm/include/llvm/IR/DataLayout.h

enum class FunctionPtrAlignType {
     /// The function pointer alignment is independent of the function alignment.
     Independent,
     /// The function pointer alignment is a multiple of the function alignment.
     MultipleOfFunctionAlign,
   };
 private:
   /// Defaults to false.
   bool BigEndian;
-
+  /// Whether vectors in this DataLayout are specified by element size or total
+  /// size. Defaults to false (total size).
+  bool VectorsByEltSize;
   unsigned AllocaAddrSpace;
   MaybeAlign StackNaturalAlign;
   unsigned ProgramAddrSpace;
   unsigned DefaultGlobalsAddrSpace;
 
   MaybeAlign FunctionPtrAlign;
   FunctionPtrAlignType TheFunctionPtrAlignType;
 
public:
   DataLayout(const DataLayout &DL) { *this = DL; }
 
   ~DataLayout(); // Not virtual, do not subclass this class
 
   DataLayout &operator=(const DataLayout &DL) {
     clear();
     StringRepresentation = DL.StringRepresentation;
     BigEndian = DL.isBigEndian();
+    VectorsByEltSize = DL.VectorsByEltSize;
     AllocaAddrSpace = DL.AllocaAddrSpace;
     StackNaturalAlign = DL.StackNaturalAlign;
     FunctionPtrAlign = DL.FunctionPtrAlign;
     TheFunctionPtrAlignType = DL.TheFunctionPtrAlignType;
     ProgramAddrSpace = DL.ProgramAddrSpace;
     DefaultGlobalsAddrSpace = DL.DefaultGlobalsAddrSpace;
     ManglingMode = DL.ManglingMode;
     LegalIntWidths = DL.LegalIntWidths;

llvm/lib/IR/DataLayout.cpp

static const LayoutAlignElem DefaultAlignments[] = {
     {AGGREGATE_ALIGN, 0, Align(1), Align(8)}   // struct
 };
 
 void DataLayout::reset(StringRef Desc) {
   clear();
 
   LayoutMap = nullptr;
   BigEndian = false;
+  VectorsByEltSize = false;
   AllocaAddrSpace = 0;
   StackNaturalAlign.reset();
   ProgramAddrSpace = 0;
   DefaultGlobalsAddrSpace = 0;
   FunctionPtrAlign.reset();
   TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
   ManglingMode = MM_None;
   NonIntegralAddressSpaces.clear();
if (Error Err = getInt(R, AddrSpace))
     return Err;
   if (!isUInt<24>(AddrSpace))
     return reportError("Invalid address space, must be a 24-bit integer");
   return Error::success();
 }
 
 Error DataLayout::parseSpecifier(StringRef Desc) {
   StringRepresentation = std::string(Desc);
+  bool ExplicitVectorsBySize = false;
   while (!Desc.empty()) {
     // Split at '-'.
     std::pair<StringRef, StringRef> Split;
     if (Error Err = ::split(Desc, '-', Split))
       return Err;
     Desc = Split.second;
 
     // Split at ':'.
case 'a': {
       switch (Specifier) {
       default: llvm_unreachable("Unexpected specifier!");
       case 'i': AlignType = INTEGER_ALIGN; break;
       case 'v': AlignType = VECTOR_ALIGN; break;
       case 'f': AlignType = FLOAT_ALIGN; break;
       case 'a': AlignType = AGGREGATE_ALIGN; break;
       }
 
+      // Check whether this is actually defining vectors by element size (ve).
+      if (AlignType == VECTOR_ALIGN) {
+        if (Tok.front() == 'e') {
+          // Vectors by element: consume the 'e'.
+          Tok = Tok.substr(1);
+          if (ExplicitVectorsBySize)
+            return reportError("Vectors must be explicitly set by total size "
+                               "(v) OR element size (ve)");
+          // If we're setting vectors-by-element, we must wipe any pre-existing
+          // vector settings: the defaults implicitly set v64 and v128 by size.
+          if (!VectorsByEltSize) {
+            VectorsByEltSize = true;
+            Alignments.erase(llvm::remove_if(Alignments,
+                                             [](const LayoutAlignElem &LE) {
+                                               return LE.AlignType ==
+                                                      VECTOR_ALIGN;
+                                             }),
+                             Alignments.end());
+          }
+        } else {
+          if (VectorsByEltSize)
+            return reportError("Vectors must be explicitly set by total size "
+                               "(v) OR element size (ve)");
+          ExplicitVectorsBySize = true;
+        }
+      }
+
       // Bit size.
       unsigned Size = 0;
       if (!Tok.empty())
         if (Error Err = getInt(Tok, Size))
           return Err;
 
       if (AlignType == AGGREGATE_ALIGN && Size != 0)
         return reportError(

 DataLayout::DataLayout(const Module *M) {
   init(M);
 }
 
 void DataLayout::init(const Module *M) { *this = M->getDataLayout(); }
 
 bool DataLayout::operator==(const DataLayout &Other) const {
   bool Ret = BigEndian == Other.BigEndian &&
+             VectorsByEltSize == Other.VectorsByEltSize &&
              AllocaAddrSpace == Other.AllocaAddrSpace &&
              StackNaturalAlign == Other.StackNaturalAlign &&
              ProgramAddrSpace == Other.ProgramAddrSpace &&
              DefaultGlobalsAddrSpace == Other.DefaultGlobalsAddrSpace &&
              FunctionPtrAlign == Other.FunctionPtrAlign &&
              TheFunctionPtrAlignType == Other.TheFunctionPtrAlignType &&
              ManglingMode == Other.ManglingMode &&
              LegalIntWidths == Other.LegalIntWidths &&
case Type::X86_FP80TyID: {
     // approximation of reality, and if the user wanted something less
     // less conservative, they should have specified it explicitly in the data
     // layout.
     return Align(PowerOf2Ceil(BitWidth / 8));
   }
   case Type::X86_MMXTyID:
   case Type::FixedVectorTyID:
   case Type::ScalableVectorTyID: {
-    unsigned BitWidth = getTypeSizeInBits(Ty).getKnownMinSize();
-    auto I = findAlignmentLowerBound(VECTOR_ALIGN, BitWidth);
+    // FIXME: Can X86_MMXTyID and VectorsByEltSize coexist?
+    Optional<unsigned> BitWidth;
+    if (VectorsByEltSize && Ty->getTypeID() != Type::X86_MMXTyID) {
+      auto *EltTy = cast<VectorType>(Ty)->getElementType();
+      BitWidth = getTypeSizeInBits(EltTy).getFixedSize();
+    } else if (!VectorsByEltSize) {
+      BitWidth = getTypeSizeInBits(Ty).getKnownMinSize();
+    }
+
+    if (BitWidth) {
+      const auto *I = findAlignmentLowerBound(VECTOR_ALIGN, *BitWidth);
       if (I != Alignments.end() && I->AlignType == VECTOR_ALIGN &&
           I->TypeBitWidth == BitWidth)
         return abi_or_pref ? I->ABIAlign : I->PrefAlign;
+    }
 
     // By default, use natural alignment for vector types. This is consistent
     // with what clang and llvm-gcc do.
     //
     // We're only calculating a natural alignment, so it doesn't have to be
     // based on the full size for scalable vectors. Using the minimum element
     // count should be enough here.
     return Align(PowerOf2Ceil(getTypeStoreSize(Ty).getKnownMinSize()));

llvm/unittests/IR/DataLayoutTest.cpp

TEST(DataLayoutTest, VectorAlign) {
   LLVMContext Context;
   Type *const FloatTy = Type::getFloatTy(Context);
   Type *const V8F32Ty = FixedVectorType::get(FloatTy, 8);
 
   // The alignment for a vector type larger than any specified vector type uses
   // the natural alignment as a fallback.
   EXPECT_EQ(Align(4 * 8), DL->getABITypeAlign(V8F32Ty));
   EXPECT_EQ(Align(4 * 8), DL->getPrefTypeAlign(V8F32Ty));
+
+  // Test vectors by total size in isolation.
+  EXPECT_THAT_EXPECTED(DataLayout::parse("v64:64"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("v64:64:64"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("v64:64-v128:128"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("v64:64-v128:128:128"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("v64:64:64-v128:128"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("v64:64:64-v128:128:128"),
+                       Succeeded());
+  // Test vectors by element in isolation.
+  EXPECT_THAT_EXPECTED(DataLayout::parse("ve8:8"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("ve8:8:8"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("ve8:8-ve32:32"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("ve8:8-ve32:32:32"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("ve8:8:8-ve32:32"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("ve8:8:8-ve32:32:32"), Succeeded());
+  // We can't mix vectors by total size and by element size.
+  EXPECT_THAT_EXPECTED(DataLayout::parse("v64:64-ve32:32"), Failed());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("ve8:8-v128:128"), Failed());
+
+  Type *const I8Ty = Type::getInt8Ty(Context);
+  Type *const I16Ty = Type::getInt16Ty(Context);
+  Type *const I32Ty = Type::getInt32Ty(Context);
+  Type *const HalfTy = Type::getHalfTy(Context);
+
+  DL = DataLayout::parse("ve8:16:32-ve16:64");
+  EXPECT_THAT_EXPECTED(DL, Succeeded());
+
+  for (unsigned NumElts : {1, 2, 3, 4, 6, 8, 16, 64, 80, 128, 1024}) {
+    auto FEC = ElementCount::getFixed(NumElts);
+    auto SEC = ElementCount::getScalable(NumElts);
+    // 8-bit exact matches.
+    auto *FixVI8Ty = VectorType::get(I8Ty, FEC);
+    auto *ScaVI8Ty = VectorType::get(I8Ty, SEC);
+    EXPECT_EQ(Align(2), DL->getABITypeAlign(FixVI8Ty));
+    EXPECT_EQ(Align(4), DL->getPrefTypeAlign(FixVI8Ty));
+    EXPECT_EQ(Align(2), DL->getABITypeAlign(ScaVI8Ty));
+    EXPECT_EQ(Align(4), DL->getPrefTypeAlign(ScaVI8Ty));
+
+    // 16-bit exact matches.
+    auto *FixVI16Ty = VectorType::get(I16Ty, FEC);
+    auto *ScaVI16Ty = VectorType::get(I16Ty, SEC);
+    EXPECT_EQ(Align(8), DL->getABITypeAlign(FixVI16Ty));
+    EXPECT_EQ(Align(8), DL->getPrefTypeAlign(FixVI16Ty));
+    EXPECT_EQ(Align(8), DL->getABITypeAlign(ScaVI16Ty));
+    EXPECT_EQ(Align(8), DL->getPrefTypeAlign(ScaVI16Ty));
+
+    auto *FixVF16Ty = VectorType::get(HalfTy, FEC);
+    auto *ScaVF16Ty = VectorType::get(HalfTy, SEC);
+    EXPECT_EQ(Align(8), DL->getABITypeAlign(FixVF16Ty));
+    EXPECT_EQ(Align(8), DL->getPrefTypeAlign(FixVF16Ty));
+    EXPECT_EQ(Align(8), DL->getABITypeAlign(ScaVF16Ty));
+    EXPECT_EQ(Align(8), DL->getPrefTypeAlign(ScaVF16Ty));
+
+    // 32-bit types fall back to the natural alignment.
+    auto *FixVI32Ty = VectorType::get(I32Ty, FEC);
+    auto *ScaVI32Ty = VectorType::get(I32Ty, SEC);
+    Align ExpectedAlign(PowerOf2Ceil(NumElts * 4));
+    EXPECT_EQ(ExpectedAlign, DL->getABITypeAlign(FixVI32Ty));
+    EXPECT_EQ(ExpectedAlign, DL->getPrefTypeAlign(FixVI32Ty));
+    EXPECT_EQ(ExpectedAlign, DL->getABITypeAlign(ScaVI32Ty));
+    EXPECT_EQ(ExpectedAlign, DL->getPrefTypeAlign(ScaVI32Ty));
+
+    auto *FixVF32Ty = VectorType::get(FloatTy, FEC);
+    auto *ScaVF32Ty = VectorType::get(FloatTy, SEC);
+    EXPECT_EQ(ExpectedAlign, DL->getABITypeAlign(FixVF32Ty));
+    EXPECT_EQ(ExpectedAlign, DL->getPrefTypeAlign(FixVF32Ty));
+    EXPECT_EQ(ExpectedAlign, DL->getABITypeAlign(ScaVF32Ty));
+    EXPECT_EQ(ExpectedAlign, DL->getPrefTypeAlign(ScaVF32Ty));
+  }
 }
 
 } // anonymous namespace