[Alignment][NFC] Attributes use Align/MaybeAlign

[gchatelet]

Summary

This is patch is part of a series to introduce an Alignment type.
See this thread for context: http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790

[courbet]

llvm/include/llvm/IR/Attributes.h

803

Please update the comment.

805–813

is not what ? :)

815

ditto

llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp

1573–1574

align < 4: do we want this to compile ?

[gchatelet]

• Address comments

llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp

1573–1574

We need to be able to compare an alignment with an offset for instance, in which case there is no power of two guarantee.
In that sense, we want this to compile.

Now in this case I agree that it should be align < Align(4)

llvm/include/llvm/IR/Attributes.h

public:
   StringRef getValueAsString() const;
 
   /// Return the attribute's value as a Type. This requires the attribute to be
   /// a type attribute.
   Type *getValueAsType() const;
 
   /// Returns the alignment field of an attribute as a byte alignment
   /// value.
-  unsigned getAlignment() const;
+  MaybeAlign getAlignment() const;
 
   /// Returns the stack alignment field of an attribute as a byte
   /// alignment value.
-  unsigned getStackAlignment() const;
+  MaybeAlign getStackAlignment() const;
 
   /// Returns the number of dereferenceable bytes from the
   /// dereferenceable attribute.
   uint64_t getDereferenceableBytes() const;
 
   /// Returns the number of dereferenceable_or_null bytes from the
   /// dereferenceable_or_null attribute.
   uint64_t getDereferenceableOrNullBytes() const;
public:
   bool hasAttribute(StringRef Kind) const;
 
   /// Return the attribute object.
   Attribute getAttribute(Attribute::AttrKind Kind) const;
 
   /// Return the target-dependent attribute object.
   Attribute getAttribute(StringRef Kind) const;
 
-  unsigned getAlignment() const;
-  unsigned getStackAlignment() const;
+  MaybeAlign getAlignment() const;
+  MaybeAlign getStackAlignment() const;
   uint64_t getDereferenceableBytes() const;
   uint64_t getDereferenceableOrNullBytes() const;
   Type *getByValType() const;
   std::pair<unsigned, Optional<unsigned>> getAllocSizeArgs() const;
   std::string getAsString(bool InAttrGrp = false) const;
 
   using iterator = const Attribute *;
 
public:
   }
 
   /// Return the attribute object that exists at the given index.
   Attribute getParamAttr(unsigned ArgNo, StringRef Kind) const {
     return getAttribute(ArgNo + FirstArgIndex, Kind);
   }
 
   /// Return the alignment of the return value.
-  unsigned getRetAlignment() const;
+  MaybeAlign getRetAlignment() const;
 
   /// Return the alignment for the specified function parameter.
-  unsigned getParamAlignment(unsigned ArgNo) const;
+  MaybeAlign getParamAlignment(unsigned ArgNo) const;
 
   /// Return the byval type for the specified function parameter.
   Type *getParamByValType(unsigned ArgNo) const;
 
   /// Get the stack alignment.
-  unsigned getStackAlignment(unsigned Index) const;
+  MaybeAlign getStackAlignment(unsigned Index) const;
 
   /// Get the number of dereferenceable bytes (or zero if unknown).
   uint64_t getDereferenceableBytes(unsigned Index) const;
 
   /// Get the number of dereferenceable bytes (or zero if unknown) of an
   /// arg.
   uint64_t getParamDereferenceableBytes(unsigned ArgNo) const {
     return getDereferenceableBytes(ArgNo + FirstArgIndex);
public:
 
   /// Retrieve the byval type.
   Type *getByValType() const { return ByValType; }
 
   /// Retrieve the allocsize args, if the allocsize attribute exists.  If it
   /// doesn't exist, pair(0, 0) is returned.
   std::pair<unsigned, Optional<unsigned>> getAllocSizeArgs() const;
 
+  /// This turns an alignment into the form used internally in Attribute.
+  /// This call has no effect if Align is not set.
+  AttrBuilder &addAlignmentAttr(MaybeAlign Align);
+
   /// This turns an int alignment (which must be a power of 2) into the
   /// form used internally in Attribute.
-  AttrBuilder &addAlignmentAttr(unsigned Align);
+  /// This call has no effect if Align is 0.
+  /// Deprecated, use the version using a MaybeAlign.
+  inline AttrBuilder &addAlignmentAttr(unsigned Align) {
+    return addAlignmentAttr(MaybeAlign(Align));
+  }
+
+  /// This turns a stack alignment into the form used internally in Attribute.
+  /// This call has no effect if Align is not set.
+  AttrBuilder &addStackAlignmentAttr(MaybeAlign Align);
 
   /// This turns an int stack alignment (which must be a power of 2) into
   /// the form used internally in Attribute.
-  AttrBuilder &addStackAlignmentAttr(unsigned Align);
+  /// This call has no effect if Align is 0.
+  /// Deprecated, use the version using a MaybeAlign.
+  inline AttrBuilder &addStackAlignmentAttr(unsigned Align) {
+    return addStackAlignmentAttr(MaybeAlign(Align));
+  }
 
   /// This turns the number of dereferenceable bytes into the form used
   /// internally in Attribute.
   AttrBuilder &addDereferenceableAttr(uint64_t Bytes);
 
   /// This turns the number of dereferenceable_or_null bytes into the
   /// form used internally in Attribute.
   AttrBuilder &addDereferenceableOrNullAttr(uint64_t Bytes);

llvm/include/llvm/IR/Function.h

public:
   Attribute getFnAttribute(StringRef Kind) const {
     return getAttribute(AttributeList::FunctionIndex, Kind);
   }
 
   /// Return the stack alignment for the function.
   unsigned getFnStackAlignment() const {
     if (!hasFnAttribute(Attribute::StackAlignment))
       return 0;
-    return AttributeSets.getStackAlignment(AttributeList::FunctionIndex);
+    if (const auto MA =
+            AttributeSets.getStackAlignment(AttributeList::FunctionIndex))
+      return MA->value();
+    return 0;
   }
 
   /// hasGC/getGC/setGC/clearGC - The name of the garbage collection algorithm
   ///                             to use during code generation.
   bool hasGC() const {
     return getSubclassDataFromValue() & (1<<14);
   }
   const std::string &getGC() const;
public:
   void addDereferenceableOrNullAttr(unsigned i, uint64_t Bytes);
 
   /// adds the dereferenceable_or_null attribute to the list of
   /// attributes for the given arg.
   void addDereferenceableOrNullParamAttr(unsigned ArgNo, uint64_t Bytes);
 
   /// Extract the alignment for a call or parameter (0=unknown).
   unsigned getParamAlignment(unsigned ArgNo) const {
-    return AttributeSets.getParamAlignment(ArgNo);
+    if (const auto MA = AttributeSets.getParamAlignment(ArgNo))
+      return MA->value();
+    return 0;
   }
 
   /// Extract the byval type for a parameter.
   Type *getParamByValType(unsigned ArgNo) const {
     Type *Ty = AttributeSets.getParamByValType(ArgNo);
     return Ty ? Ty : (arg_begin() + ArgNo)->getType()->getPointerElementType();
   }
 

llvm/include/llvm/IR/InstrTypes.h

public:
   // FIXME: Once this API is no longer duplicated in `CallSite`, rename this to
   // better indicate that this may return a conservative answer.
   bool doesNotReadMemory(unsigned OpNo) const {
     return dataOperandHasImpliedAttr(OpNo + 1, Attribute::WriteOnly) ||
            dataOperandHasImpliedAttr(OpNo + 1, Attribute::ReadNone);
   }
 
   /// Extract the alignment of the return value.
-  unsigned getRetAlignment() const { return Attrs.getRetAlignment(); }
+  unsigned getRetAlignment() const {
+    if (const auto MA = Attrs.getRetAlignment())
+      return MA->value();
+    return 0;
+  }
 
   /// Extract the alignment for a call or parameter (0=unknown).
   unsigned getParamAlignment(unsigned ArgNo) const {
-    return Attrs.getParamAlignment(ArgNo);
+    if (const auto MA = Attrs.getParamAlignment(ArgNo))
+      return MA->value();
+    return 0;
   }
 
   /// Extract the byval type for a call or parameter.
   Type *getParamByValType(unsigned ArgNo) const {
     Type *Ty = Attrs.getParamByValType(ArgNo);
     return Ty ? Ty : getArgOperand(ArgNo)->getType()->getPointerElementType();
   }
 

llvm/lib/IR/AttributeImpl.h

bool hasAttribute(Attribute::AttrKind Kind) const {
     return AvailableAttrs[Kind / 8] & ((uint64_t)1) << (Kind % 8);
   }
   bool hasAttribute(StringRef Kind) const;
   bool hasAttributes() const { return NumAttrs != 0; }
 
   Attribute getAttribute(Attribute::AttrKind Kind) const;
   Attribute getAttribute(StringRef Kind) const;
 
-  unsigned getAlignment() const;
-  unsigned getStackAlignment() const;
+  MaybeAlign getAlignment() const;
+  MaybeAlign getStackAlignment() const;
   uint64_t getDereferenceableBytes() const;
   uint64_t getDereferenceableOrNullBytes() const;
   std::pair<unsigned, Optional<unsigned>> getAllocSizeArgs() const;
   std::string getAsString(bool InAttrGrp) const;
   Type *getByValType() const;
 
   using iterator = const Attribute *;
 

llvm/lib/IR/Attributes.cpp

bool Attribute::hasAttribute(AttrKind Kind) const {
   return (pImpl && pImpl->hasAttribute(Kind)) || (!pImpl && Kind == None);
 }
 
 bool Attribute::hasAttribute(StringRef Kind) const {
   if (!isStringAttribute()) return false;
   return pImpl && pImpl->hasAttribute(Kind);
 }
 
-unsigned Attribute::getAlignment() const {
+MaybeAlign Attribute::getAlignment() const {
   assert(hasAttribute(Attribute::Alignment) &&
          "Trying to get alignment from non-alignment attribute!");
-  return pImpl->getValueAsInt();
+  return MaybeAlign(pImpl->getValueAsInt());
 }
 
-unsigned Attribute::getStackAlignment() const {
+MaybeAlign Attribute::getStackAlignment() const {
   assert(hasAttribute(Attribute::StackAlignment) &&
          "Trying to get alignment from non-alignment attribute!");
-  return pImpl->getValueAsInt();
+  return MaybeAlign(pImpl->getValueAsInt());
 }
 
 uint64_t Attribute::getDereferenceableBytes() const {
   assert(hasAttribute(Attribute::Dereferenceable) &&
          "Trying to get dereferenceable bytes from "
          "non-dereferenceable attribute!");
   return pImpl->getValueAsInt();
 }

 Attribute AttributeSet::getAttribute(Attribute::AttrKind Kind) const {
   return SetNode ? SetNode->getAttribute(Kind) : Attribute();
 }
 
 Attribute AttributeSet::getAttribute(StringRef Kind) const {
   return SetNode ? SetNode->getAttribute(Kind) : Attribute();
 }
 
-unsigned AttributeSet::getAlignment() const {
-  return SetNode ? SetNode->getAlignment() : 0;
+MaybeAlign AttributeSet::getAlignment() const {
+  return SetNode ? SetNode->getAlignment() : None;
 }
 
-unsigned AttributeSet::getStackAlignment() const {
-  return SetNode ? SetNode->getStackAlignment() : 0;
+MaybeAlign AttributeSet::getStackAlignment() const {
+  return SetNode ? SetNode->getStackAlignment() : None;
 }
 
 uint64_t AttributeSet::getDereferenceableBytes() const {
   return SetNode ? SetNode->getDereferenceableBytes() : 0;
 }
 
 uint64_t AttributeSet::getDereferenceableOrNullBytes() const {
   return SetNode ? SetNode->getDereferenceableOrNullBytes() : 0;

 
 Attribute AttributeSetNode::getAttribute(StringRef Kind) const {
   for (const auto I : *this)
     if (I.hasAttribute(Kind))
       return I;
   return {};
 }
 
-unsigned AttributeSetNode::getAlignment() const {
+MaybeAlign AttributeSetNode::getAlignment() const {
   for (const auto I : *this)
     if (I.hasAttribute(Attribute::Alignment))
       return I.getAlignment();
-  return 0;
+  return None;
 }
 
-unsigned AttributeSetNode::getStackAlignment() const {
+MaybeAlign AttributeSetNode::getStackAlignment() const {
   for (const auto I : *this)
     if (I.hasAttribute(Attribute::StackAlignment))
       return I.getStackAlignment();
-  return 0;
+  return None;
 }
 
 Type *AttributeSetNode::getByValType() const {
   for (const auto I : *this)
     if (I.hasAttribute(Attribute::ByVal))
       return I.getValueAsType();
   return 0;
 }
if (!B.hasAttributes())
     return *this;
 
   if (!pImpl)
     return AttributeList::get(C, {{Index, AttributeSet::get(C, B)}});
 
 #ifndef NDEBUG
   // FIXME it is not obvious how this should work for alignment. For now, say
   // we can't change a known alignment.
-  unsigned OldAlign = getAttributes(Index).getAlignment();
+  const MaybeAlign OldAlign = getAttributes(Index).getAlignment();
   unsigned NewAlign = B.getAlignment();
   assert((!OldAlign || !NewAlign || OldAlign == NewAlign) &&
          "Attempt to change alignment!");
 #endif
 
   Index = attrIdxToArrayIdx(Index);
   SmallVector<AttributeSet, 4> AttrSets(this->begin(), this->end());
   if (Index >= AttrSets.size())
Attribute AttributeList::getAttribute(unsigned Index,
                                       Attribute::AttrKind Kind) const {
   return getAttributes(Index).getAttribute(Kind);
 }
 
 Attribute AttributeList::getAttribute(unsigned Index, StringRef Kind) const {
   return getAttributes(Index).getAttribute(Kind);
 }
 
-unsigned AttributeList::getRetAlignment() const {
+MaybeAlign AttributeList::getRetAlignment() const {
   return getAttributes(ReturnIndex).getAlignment();
 }
 
-unsigned AttributeList::getParamAlignment(unsigned ArgNo) const {
+MaybeAlign AttributeList::getParamAlignment(unsigned ArgNo) const {
   return getAttributes(ArgNo + FirstArgIndex).getAlignment();
 }
 
 Type *AttributeList::getParamByValType(unsigned Index) const {
   return getAttributes(Index+FirstArgIndex).getByValType();
 }
 
-
-unsigned AttributeList::getStackAlignment(unsigned Index) const {
+MaybeAlign AttributeList::getStackAlignment(unsigned Index) const {
   return getAttributes(Index).getStackAlignment();
 }
 
 uint64_t AttributeList::getDereferenceableBytes(unsigned Index) const {
   return getAttributes(Index).getDereferenceableBytes();
 }
 
 uint64_t AttributeList::getDereferenceableOrNullBytes(unsigned Index) const {
if (I != TargetDepAttrs.end())
     TargetDepAttrs.erase(I);
   return *this;
 }
 
 std::pair<unsigned, Optional<unsigned>> AttrBuilder::getAllocSizeArgs() const {
   return unpackAllocSizeArgs(AllocSizeArgs);
 }
 
-AttrBuilder &AttrBuilder::addAlignmentAttr(unsigned A) {
-  MaybeAlign Align(A);
+AttrBuilder &AttrBuilder::addAlignmentAttr(MaybeAlign Align) {
   if (!Align)
     return *this;
 
   assert(*Align <= 0x40000000 && "Alignment too large.");
 
   Attrs[Attribute::Alignment] = true;
   Alignment = Align;
   return *this;
 }
 
-AttrBuilder &AttrBuilder::addStackAlignmentAttr(unsigned A) {
-  MaybeAlign Align(A);
+AttrBuilder &AttrBuilder::addStackAlignmentAttr(MaybeAlign Align) {
   // Default alignment, allow the target to define how to align it.
   if (!Align)
     return *this;
 
   assert(*Align <= 0x100 && "Alignment too large.");
 
   Attrs[Attribute::StackAlignment] = true;
   StackAlignment = Align;

llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp

if (isKernelFunction(*F)) {
       }
     }
 
     if (!PAL.hasParamAttribute(paramIndex, Attribute::ByVal)) {
       if (Ty->isAggregateType() || Ty->isVectorTy() || Ty->isIntegerTy(128)) {
         // Just print .param .align <a> .b8 .param[size];
         // <a> = PAL.getparamalignment
         // size = typeallocsize of element type
-        unsigned align = PAL.getParamAlignment(paramIndex);
-        if (align == 0)
-          align = DL.getABITypeAlignment(Ty);
+        const Align align = DL.getValueOrABITypeAlignment(
+            PAL.getParamAlignment(paramIndex), Ty);
 
         unsigned sz = DL.getTypeAllocSize(Ty);
-        O << "\t.param .align " << align << " .b8 ";
+        O << "\t.param .align " << align.value() << " .b8 ";
         printParamName(I, paramIndex, O);
         O << "[" << sz << "]";
 
         continue;
       }
       // Just a scalar
       auto *PTy = dyn_cast<PointerType>(Ty);
       if (isKernelFunc) {
for (I = F->arg_begin(), E = F->arg_end(); I != E; ++I, paramIndex++) {
     auto *PTy = dyn_cast<PointerType>(Ty);
     assert(PTy && "Param with byval attribute should be a pointer type");
     Type *ETy = PTy->getElementType();
 
     if (isABI || isKernelFunc) {
       // Just print .param .align <a> .b8 .param[size];
       // <a> = PAL.getparamalignment
       // size = typeallocsize of element type
-      unsigned align = PAL.getParamAlignment(paramIndex);
-      if (align == 0)
-        align = DL.getABITypeAlignment(ETy);
+      Align align =
+          DL.getValueOrABITypeAlignment(PAL.getParamAlignment(paramIndex), ETy);
       // Work around a bug in ptxas. When PTX code takes address of
       // byval parameter with alignment < 4, ptxas generates code to
       // spill argument into memory. Alas on sm_50+ ptxas generates
       // SASS code that fails with misaligned access. To work around
       // the problem, make sure that we align byval parameters by at
       // least 4. Matching change must be made in LowerCall() where we
       // prepare parameters for the call.
       //
       // TODO: this will need to be undone when we get to support multi-TU
       // device-side compilation as it breaks ABI compatibility with nvcc.
       // Hopefully ptxas bug is fixed by then.
-      if (!isKernelFunc && align < 4)
-        align = 4;
+      if (!isKernelFunc && align < Align(4))
+        align = Align(4);
       unsigned sz = DL.getTypeAllocSize(ETy);
-      O << "\t.param .align " << align << " .b8 ";
+      O << "\t.param .align " << align.value() << " .b8 ";
       printParamName(I, paramIndex, O);
       O << "[" << sz << "]";
       continue;
     } else {
       // Split the ETy into constituent parts and
       // print .param .b<size> <name> for each part.
       // Further, if a part is vector, print the above for
       // each vector element.

llvm/unittests/IR/AttributesTest.cpp

 
 TEST(Attributes, AddMatchingAlignAttr) {
   LLVMContext C;
   AttributeList AL;
   AL = AL.addAttribute(C, AttributeList::FirstArgIndex,
                        Attribute::getWithAlignment(C, Align(8)));
   AL = AL.addAttribute(C, AttributeList::FirstArgIndex + 1,
                        Attribute::getWithAlignment(C, Align(32)));
-  EXPECT_EQ(8U, AL.getParamAlignment(0));
-  EXPECT_EQ(32U, AL.getParamAlignment(1));
+  EXPECT_EQ(Align(8), AL.getParamAlignment(0));
+  EXPECT_EQ(Align(32), AL.getParamAlignment(1));
 
   AttrBuilder B;
   B.addAttribute(Attribute::NonNull);
   B.addAlignmentAttr(8);
   AL = AL.addAttributes(C, AttributeList::FirstArgIndex, B);
-  EXPECT_EQ(8U, AL.getParamAlignment(0));
-  EXPECT_EQ(32U, AL.getParamAlignment(1));
+  EXPECT_EQ(Align(8), AL.getParamAlignment(0));
+  EXPECT_EQ(Align(32), AL.getParamAlignment(1));
   EXPECT_TRUE(AL.hasParamAttribute(0, Attribute::NonNull));
 }
 
 TEST(Attributes, EmptyGet) {
   LLVMContext C;
   AttributeList EmptyLists[] = {AttributeList(), AttributeList()};
   AttributeList AL = AttributeList::get(C, EmptyLists);
   EXPECT_TRUE(AL.isEmpty());