Allow DataLayout to support arbitrary pointer sizes

[stephenneuendorffer]

Summary

Currently, it is impossible to specify a DataLayout with pointer
size and index size that is not a whole number of bytes.
This patch modifies
the DataLayout class to accept arbitrary pointer sizes and to
store the size as a number of bits, rather than as a number of bytes.
Generally speaking, the external interface of the class as used
by in-tree architectures remains the same and shouldn't affect the
behavior of architecures with pointer sizes equal to a whole number
of bytes.

Note the interface of setPointerAlignment has changed and takes
a pointer and index size that is a number of bits, rather than a number
of bytes.

Patch originally by Ajit Kumar Agarwal

[jrtc27]

llvm/lib/IR/DataLayout.cpp

321

These shouldn't need an explicit template parameter, it's inferable from PointerMemSize's type?

714

divideCeil from MathExtras?

llvm/lib/Target/M68k/M68kISelLowering.cpp

173

These aren't really necessary. I'd suggest separating out the "add more flexibility to DataLayout" changes from the "simplify users of existing interfaces by using new ones" changes so it's clear there isn't actually an API break here.

[tschuett]

llvm/include/llvm/IR/DataLayout.h

101–102

This is dangerous. You are changing the meaning without changing the names?!? Maybe 2 versions of get?

[tschuett]

llvm/include/llvm/IR/DataLayout.h

183

Same here. Maybe keep the old functions and only add new functions with ìnBits in the name.

[stephenneuendorffer]

llvm/include/llvm/IR/DataLayout.h

183

True, but this seems like a private interface? I'd be more inclined to change the name without keeping the old function.

[jeroen.dobbelaere]

Not sure how you can construct a test that triggers this problem, but in our out-of-tree version, this triggered problems.

llvm/lib/IR/DataLayout.cpp

714

Shouldn't this be .....divideCeil(P.IndexBitWidth, 8) ?

[uabelho]

llvm/lib/IR/DataLayout.cpp

714

Ping @stephenneuendorffer

[jeroen.dobbelaere]

ping2 @stephenneuendorffer : could you check and comment on P.TypeBitWidth vs P.IndexBitWidth on line 715 ?

llvm/include/llvm/IR/DataLayout.h

 ///
 /// Stores the alignment data associated with a given pointer and address space.
 ///
 /// \note The unusual order of elements in the structure attempts to reduce
 /// padding and make the structure slightly more cache friendly.
 struct PointerAlignElem {
   Align ABIAlign;
   Align PrefAlign;
-  uint32_t TypeByteWidth;
+  uint32_t TypeBitWidth;
   uint32_t AddressSpace;
-  uint32_t IndexWidth;
+  uint32_t IndexBitWidth;
 
   /// Initializer
   static PointerAlignElem get(uint32_t AddressSpace, Align ABIAlign,
-                              Align PrefAlign, uint32_t TypeByteWidth,
-                              uint32_t IndexWidth);
+                              Align PrefAlign, uint32_t TypeBitWidth,
+                              uint32_t IndexBitWidth);
 
   bool operator==(const PointerAlignElem &rhs) const;
 };
 
 /// A parsed version of the target data layout string in and methods for
 /// querying it.
 ///
 /// The target data layout string is specified *by the target* - a frontend
private:
 
   /// Attempts to set the alignment of the given type. Returns an error
   /// description on failure.
   Error setAlignment(AlignTypeEnum align_type, Align abi_align,
                      Align pref_align, uint32_t bit_width);
 
   /// Attempts to set the alignment of a pointer in the given address space.
   /// Returns an error description on failure.
   Error setPointerAlignment(uint32_t AddrSpace, Align ABIAlign, Align PrefAlign,
-                            uint32_t TypeByteWidth, uint32_t IndexWidth);
+                            uint32_t TypeBitWidth, uint32_t IndexBitWidth);
 
   /// Internal helper to get alignment for integer of given bitwidth.
   Align getIntegerAlignment(uint32_t BitWidth, bool abi_or_pref) const;
 
   /// Internal helper method that returns requested alignment for type.
   Align getAlignment(Type *Ty, bool abi_or_pref) const;
 
   /// Attempts to parse a target data specification string and reports an error
public:
   /// Layout pointer alignment
   Align getPointerABIAlignment(unsigned AS) const;
 
   /// Return target's alignment for stack-based pointers
   /// FIXME: The defaults need to be removed once all of
   /// the backends/clients are updated.
   Align getPointerPrefAlignment(unsigned AS = 0) const;
 
-  /// Layout pointer size
+  /// Layout pointer size in bytes, rounded up to a whole
+  /// number of bytes.
   /// FIXME: The defaults need to be removed once all of
   /// the backends/clients are updated.
   unsigned getPointerSize(unsigned AS = 0) const;
 
   /// Returns the maximum index size over all address spaces.
   unsigned getMaxIndexSize() const;
 
-  // Index size used for address calculation.
+  // Index size in bytes used for address calculation,
+  /// rounded up to a whole number of bytes.
   unsigned getIndexSize(unsigned AS) const;
 
   /// Return the address spaces containing non-integral pointers.  Pointers in
   /// this address space don't have a well-defined bitwise representation.
   ArrayRef<unsigned> getNonIntegralAddressSpaces() const {
     return NonIntegralAddressSpaces;
   }
 
bool isNonIntegralPointerType(Type *Ty) const {
     auto *PTy = dyn_cast<PointerType>(Ty);
     return PTy && isNonIntegralPointerType(PTy);
   }
 
   /// Layout pointer size, in bits
   /// FIXME: The defaults need to be removed once all of
   /// the backends/clients are updated.
   unsigned getPointerSizeInBits(unsigned AS = 0) const {
-    return getPointerSize(AS) * 8;
+    return getPointerAlignElem(AS).TypeBitWidth;
   }
 
   /// Returns the maximum index size over all address spaces.
   unsigned getMaxIndexSizeInBits() const {
     return getMaxIndexSize() * 8;
   }
 
   /// Size in bits of index used for address calculation in getelementptr.
   unsigned getIndexSizeInBits(unsigned AS) const {
-    return getIndexSize(AS) * 8;
+    return getPointerAlignElem(AS).IndexBitWidth;
   }
 
   /// Layout pointer size, in bits, based on the type.  If this function is
   /// called with a pointer type, then the type size of the pointer is returned.
   /// If this function is called with a vector of pointers, then the type size
   /// of the pointer is returned.  This should only be called with a pointer or
   /// vector of pointers.
   unsigned getPointerTypeSizeInBits(Type *) const;

llvm/lib/IR/DataLayout.cpp

return (AlignType == rhs.AlignType
           && TypeBitWidth == rhs.TypeBitWidth);
 }
 
 //===----------------------------------------------------------------------===//
 // PointerAlignElem, PointerAlign support
 //===----------------------------------------------------------------------===//
 
 PointerAlignElem PointerAlignElem::get(uint32_t AddressSpace, Align ABIAlign,
-                                       Align PrefAlign, uint32_t TypeByteWidth,
-                                       uint32_t IndexWidth) {
+                                       Align PrefAlign, uint32_t TypeBitWidth,
+                                       uint32_t IndexBitWidth) {
   assert(ABIAlign <= PrefAlign && "Preferred alignment worse than ABI!");
   PointerAlignElem retval;
   retval.AddressSpace = AddressSpace;
   retval.ABIAlign = ABIAlign;
   retval.PrefAlign = PrefAlign;
-  retval.TypeByteWidth = TypeByteWidth;
-  retval.IndexWidth = IndexWidth;
+  retval.TypeBitWidth = TypeBitWidth;
+  retval.IndexBitWidth = IndexBitWidth;
   return retval;
 }
 
 bool
 PointerAlignElem::operator==(const PointerAlignElem &rhs) const {
   return (ABIAlign == rhs.ABIAlign
           && AddressSpace == rhs.AddressSpace
           && PrefAlign == rhs.PrefAlign
-          && TypeByteWidth == rhs.TypeByteWidth
-          && IndexWidth == rhs.IndexWidth);
+          && TypeBitWidth == rhs.TypeBitWidth
+          && IndexBitWidth == rhs.IndexBitWidth);
 }
 
 //===----------------------------------------------------------------------===//
 //                       DataLayout Class Implementation
 //===----------------------------------------------------------------------===//
 
 const char *DataLayout::getManglingComponent(const Triple &T) {
   if (T.isOSBinFormatGOFF())
void DataLayout::reset(StringRef Desc) {
   NonIntegralAddressSpaces.clear();
 
   // Default alignments
   for (const LayoutAlignElem &E : DefaultAlignments) {
     if (Error Err = setAlignment((AlignTypeEnum)E.AlignType, E.ABIAlign,
                                  E.PrefAlign, E.TypeBitWidth))
       return report_fatal_error(std::move(Err));
   }
-  if (Error Err = setPointerAlignment(0, Align(8), Align(8), 8, 8))
+  if (Error Err = setPointerAlignment(0, Align(8), Align(8), 64, 64))
     return report_fatal_error(std::move(Err));
 
   if (Error Err = parseSpecifier(Desc))
     return report_fatal_error(std::move(Err));
 }
 
 Expected<DataLayout> DataLayout::parse(StringRef LayoutDescription) {
   DataLayout Layout("");
case 'p': {
 
       // Size.
       if (Rest.empty())
         return reportError(
             "Missing size specification for pointer in datalayout string");
       if (Error Err = ::split(Rest, ':', Split))
         return Err;
       unsigned PointerMemSize;
-      if (Error Err = getIntInBytes(Tok, PointerMemSize))
+      if (Error Err = getInt<unsigned>(Tok, PointerMemSize))
         return Err;
       if (!PointerMemSize)
         return reportError("Invalid pointer size of 0 bytes");
 
       // ABI alignment.
       if (Rest.empty())
         return reportError(
             "Missing alignment specification for pointer in datalayout string");
case 'p': {
         if (!isPowerOf2_64(PointerPrefAlign))
           return reportError(
               "Pointer preferred alignment must be a power of 2");
 
         // Now read the index. It is the second optional parameter here.
         if (!Rest.empty()) {
           if (Error Err = ::split(Rest, ':', Split))
             return Err;
-          if (Error Err = getIntInBytes(Tok, IndexSize))
+          if (Error Err = getInt<unsigned>(Tok, IndexSize))
             return Err;
           if (!IndexSize)
             return reportError("Invalid index size of 0 bytes");
         }
       }
       if (Error Err = setPointerAlignment(
               AddrSpace, assumeAligned(PointerABIAlign),
               assumeAligned(PointerPrefAlign), PointerMemSize, IndexSize))
if (I != Pointers.end() && I->AddressSpace == AddressSpace)
       return *I;
   }
 
   assert(Pointers[0].AddressSpace == 0);
   return Pointers[0];
 }
 
 Error DataLayout::setPointerAlignment(uint32_t AddrSpace, Align ABIAlign,
-                                      Align PrefAlign, uint32_t TypeByteWidth,
-                                      uint32_t IndexWidth) {
+                                      Align PrefAlign, uint32_t TypeBitWidth,
+                                      uint32_t IndexBitWidth) {
   if (PrefAlign < ABIAlign)
     return reportError(
         "Preferred alignment cannot be less than the ABI alignment");
 
   auto I = lower_bound(Pointers, AddrSpace,
                        [](const PointerAlignElem &A, uint32_t AddressSpace) {
     return A.AddressSpace < AddressSpace;
   });
   if (I == Pointers.end() || I->AddressSpace != AddrSpace) {
     Pointers.insert(I, PointerAlignElem::get(AddrSpace, ABIAlign, PrefAlign,
-                                             TypeByteWidth, IndexWidth));
+                                             TypeBitWidth, IndexBitWidth));
   } else {
     I->ABIAlign = ABIAlign;
     I->PrefAlign = PrefAlign;
-    I->TypeByteWidth = TypeByteWidth;
-    I->IndexWidth = IndexWidth;
+    I->TypeBitWidth = TypeBitWidth;
+    I->IndexBitWidth = IndexBitWidth;
   }
   return Error::success();
 }
 
 Align DataLayout::getIntegerAlignment(uint32_t BitWidth,
                                       bool abi_or_pref) const {
   auto I = findAlignmentLowerBound(INTEGER_ALIGN, BitWidth);
   // If we don't have an exact match, use alignment of next larger integer
Align DataLayout::getPointerABIAlignment(unsigned AS) const {
   return getPointerAlignElem(AS).ABIAlign;
 }
 
 Align DataLayout::getPointerPrefAlignment(unsigned AS) const {
   return getPointerAlignElem(AS).PrefAlign;
 }
 
 unsigned DataLayout::getPointerSize(unsigned AS) const {
-  return getPointerAlignElem(AS).TypeByteWidth;
+  return (getPointerAlignElem(AS).TypeBitWidth+7)/8;
 }
 
 unsigned DataLayout::getMaxIndexSize() const {
   unsigned MaxIndexSize = 0;
   for (auto &P : Pointers)
-    MaxIndexSize = std::max(MaxIndexSize, P.IndexWidth);
+    MaxIndexSize = std::max(MaxIndexSize,
+    (P.TypeBitWidth+7)/8);
 
   return MaxIndexSize;
 }
 
 unsigned DataLayout::getPointerTypeSizeInBits(Type *Ty) const {
   assert(Ty->isPtrOrPtrVectorTy() &&
          "This should only be called with a pointer or pointer vector type");
   Ty = Ty->getScalarType();
   return getPointerSizeInBits(cast<PointerType>(Ty)->getAddressSpace());
 }
 
 unsigned DataLayout::getIndexSize(unsigned AS) const {
-  return getPointerAlignElem(AS).IndexWidth;
+  return (getPointerAlignElem(AS).IndexBitWidth+7/8);
 }
 
 unsigned DataLayout::getIndexTypeSizeInBits(Type *Ty) const {
   assert(Ty->isPtrOrPtrVectorTy() &&
          "This should only be called with a pointer or pointer vector type");
   Ty = Ty->getScalarType();
   return getIndexSizeInBits(cast<PointerType>(Ty)->getAddressSpace());
 }

llvm/lib/Target/M68k/M68kISelLowering.cpp

EVT M68kTargetLowering::getSetCCResultType(const DataLayout &DL,
   return MVT::i8;
 }
 
 MVT M68kTargetLowering::getScalarShiftAmountTy(const DataLayout &DL,
                                                EVT Ty) const {
   if (Ty.isSimple()) {
     return Ty.getSimpleVT();
   }
-  return MVT::getIntegerVT(8 * DL.getPointerSize(0));
+  return MVT::getIntegerVT(DL.getPointerSizeInBits(0));
 }
 
 #include "M68kGenCallingConv.inc"
 
 enum StructReturnType { NotStructReturn, RegStructReturn, StackStructReturn };
 
 static StructReturnType
 callIsStructReturn(const SmallVectorImpl<ISD::OutputArg> &Outs) {

llvm/lib/Target/Sparc/SparcISelLowering.cpp

static SPCC::CondCodes FPCondCCodeToFCC(ISD::CondCode CC) {
   case ISD::SETONE: return SPCC::FCC_LG;
   case ISD::SETUEQ: return SPCC::FCC_UE;
   }
 }
 
 SparcTargetLowering::SparcTargetLowering(const TargetMachine &TM,
                                          const SparcSubtarget &STI)
     : TargetLowering(TM), Subtarget(&STI) {
-  MVT PtrVT = MVT::getIntegerVT(8 * TM.getPointerSize(0));
+  MVT PtrVT = MVT::getIntegerVT(TM.getPointerSizeInBits(0));
 
   // Instructions which use registers as conditionals examine all the
   // bits (as does the pseudo SELECT_CC expansion). I don't think it
   // matters much whether it's ZeroOrOneBooleanContent, or
   // ZeroOrNegativeOneBooleanContent, so, arbitrarily choose the
   // former.
   setBooleanContents(ZeroOrOneBooleanContent);
   setBooleanVectorContents(ZeroOrOneBooleanContent);

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

static MachineOperand earlyUseOperand(MachineOperand Op) {
   if (Op.isReg())
     Op.setIsKill(false);
   return Op;
 }
 
 SystemZTargetLowering::SystemZTargetLowering(const TargetMachine &TM,
                                              const SystemZSubtarget &STI)
     : TargetLowering(TM), Subtarget(STI) {
-  MVT PtrVT = MVT::getIntegerVT(8 * TM.getPointerSize(0));
+  MVT PtrVT = MVT::getIntegerVT(TM.getPointerSizeInBits(0));
 
   auto *Regs = STI.getSpecialRegisters();
 
   // Set up the register classes.
   if (Subtarget.hasHighWord())
     addRegisterClass(MVT::i32, &SystemZ::GRX32BitRegClass);
   else
     addRegisterClass(MVT::i32, &SystemZ::GR32BitRegClass);

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) {
     EmitFeature(KV.Key);
   }
   // This pseudo-feature tells the linker whether shared memory would be safe
   EmitFeature("shared-mem");
 
   // This is an "architecture", not a "feature", but we emit it as such for
   // the benefit of tools like Binaryen and consistency with other producers.
   // FIXME: Subtarget is null here, so can't Subtarget->hasAddr64() ?
-  if (M.getDataLayout().getPointerSize() == 8) {
+  if (M.getDataLayout().getPointerSizeInBits() == 64) {
     // Can't use EmitFeature since "wasm-feature-memory64" is not a module
     // flag.
     EmittedFeatures.push_back({wasm::WASM_FEATURE_PREFIX_USED, "memory64"});
   }
 
   if (EmittedFeatures.size() == 0)
     return;
 

llvm/unittests/ExecutionEngine/JITLink/LinkGraphTests.cpp

 static ArrayRef<char> BlockContent(BlockContentBytes);
 
 TEST(LinkGraphTest, Construction) {
   // Check that LinkGraph construction works as expected.
   LinkGraph G("foo", Triple("x86_64-apple-darwin"), 8, support::little,
               getGenericEdgeKindName);
   EXPECT_EQ(G.getName(), "foo");
   EXPECT_EQ(G.getTargetTriple().str(), "x86_64-apple-darwin");
-  EXPECT_EQ(G.getPointerSize(), 8U);
+  EXPECT_EQ(G.getPointerSizeInBits(), 64U);
   EXPECT_EQ(G.getEndianness(), support::little);
   EXPECT_TRUE(llvm::empty(G.external_symbols()));
   EXPECT_TRUE(llvm::empty(G.absolute_symbols()));
   EXPECT_TRUE(llvm::empty(G.defined_symbols()));
   EXPECT_TRUE(llvm::empty(G.blocks()));
 }
 
 TEST(LinkGraphTest, AddressAccess) {