[AttrBuilder] Make handling of type attributes more generic (NFCI)

[nikic]

Summary

While working on the elementtype attribute, I felt that the type attribute handling in AttrBuilder is overly repetitive. This patch converts the separate Type* members into an std::array<Type*>, so that all type attribute kinds can be handled generically.

There's more room for improvement here (especially when it comes to converting the AttrBuilder to an Attribute), but this seems like a good starting point.

llvm/include/llvm/AsmParser/LLParser.h

private:
     bool parseStandaloneMetadata();
     bool parseNamedMetadata();
     bool parseMDString(MDString *&Result);
     bool parseMDNodeID(MDNode *&Result);
     bool parseUnnamedAttrGrp();
     bool parseFnAttributeValuePairs(AttrBuilder &B,
                                     std::vector<unsigned> &FwdRefAttrGrps,
                                     bool inAttrGrp, LocTy &BuiltinLoc);
-    bool parseRequiredTypeAttr(Type *&Result, lltok::Kind AttrName);
-    bool parsePreallocated(Type *&Result);
-    bool parseInalloca(Type *&Result);
-    bool parseByRef(Type *&Result);
+    bool parseRequiredTypeAttr(AttrBuilder &B, lltok::Kind AttrToken,
+                               Attribute::AttrKind AttrKind);
 
     // Module Summary Index Parsing.
     bool skipModuleSummaryEntry();
     bool parseSummaryEntry();
     bool parseModuleEntry(unsigned ID);
     bool parseModuleReference(StringRef &ModulePath);
     bool parseGVReference(ValueInfo &VI, unsigned &GVId);
     bool parseSummaryIndexFlags();

llvm/include/llvm/IR/Attributes.h

 
 //===----------------------------------------------------------------------===//
 /// \class
 /// This class is used in conjunction with the Attribute::get method to
 /// create an Attribute object. The object itself is uniquified. The Builder's
 /// value, however, is not. So this can be used as a quick way to test for
 /// equality, presence of attributes, etc.
 class AttrBuilder {
+  // Indices into the TypeAttrs array.
+  static const unsigned ByValTypeIndex = 0;
+  static const unsigned StructRetTypeIndex = 1;
+  static const unsigned ByRefTypeIndex = 2;
+  static const unsigned PreallocatedTypeIndex = 3;
+  static const unsigned InAllocaTypeIndex = 4;
+  static const unsigned NumTypeIndices = 5;
+
   std::bitset<Attribute::EndAttrKinds> Attrs;
   std::map<SmallString<32>, SmallString<32>, std::less<>> TargetDepAttrs;
   MaybeAlign Alignment;
   MaybeAlign StackAlignment;
   uint64_t DerefBytes = 0;
   uint64_t DerefOrNullBytes = 0;
   uint64_t AllocSizeArgs = 0;
   uint64_t VScaleRangeArgs = 0;
-  Type *ByValType = nullptr;
-  Type *StructRetType = nullptr;
-  Type *ByRefType = nullptr;
-  Type *PreallocatedType = nullptr;
-  Type *InAllocaType = nullptr;
+  std::array<Type *, NumTypeIndices> TypeAttrs = {};
+
+  Optional<unsigned> kindToTypeIndex(Attribute::AttrKind Kind) const;
 
 public:
   AttrBuilder() = default;
 
   AttrBuilder(const Attribute &A) {
     addAttribute(A);
   }
 
public:
   /// dereferenceable attribute exists (zero is returned otherwise).
   uint64_t getDereferenceableBytes() const { return DerefBytes; }
 
   /// Retrieve the number of dereferenceable_or_null bytes, if the
   /// dereferenceable_or_null attribute exists (zero is returned otherwise).
   uint64_t getDereferenceableOrNullBytes() const { return DerefOrNullBytes; }
 
   /// Retrieve the byval type.
-  Type *getByValType() const { return ByValType; }
+  Type *getByValType() const { return TypeAttrs[ByValTypeIndex]; }
 
   /// Retrieve the sret type.
-  Type *getStructRetType() const { return StructRetType; }
+  Type *getStructRetType() const { return TypeAttrs[StructRetTypeIndex]; }
 
   /// Retrieve the byref type.
-  Type *getByRefType() const { return ByRefType; }
+  Type *getByRefType() const { return TypeAttrs[ByRefTypeIndex]; }
 
   /// Retrieve the preallocated type.
-  Type *getPreallocatedType() const { return PreallocatedType; }
+  Type *getPreallocatedType() const { return TypeAttrs[PreallocatedTypeIndex]; }
 
   /// Retrieve the inalloca type.
-  Type *getInAllocaType() const { return InAllocaType; }
+  Type *getInAllocaType() const { return TypeAttrs[InAllocaTypeIndex]; }
 
   /// 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;
 
   /// Retrieve the vscale_range args, if the vscale_range attribute exists.  If
   /// it doesn't exist, pair(0, 0) is returned.
   std::pair<unsigned, unsigned> getVScaleRangeArgs() const;
public:
 
   /// This turns one (or two) ints into the form used internally in Attribute.
   AttrBuilder &addAllocSizeAttr(unsigned ElemSizeArg,
                                 const Optional<unsigned> &NumElemsArg);
 
   /// This turns two ints into the form used internally in Attribute.
   AttrBuilder &addVScaleRangeAttr(unsigned MinValue, unsigned MaxValue);
 
+  /// Add a type attribute with the given type.
+  AttrBuilder &addTypeAttr(Attribute::AttrKind Kind, Type *Ty);
+
   /// This turns a byval type into the form used internally in Attribute.
   AttrBuilder &addByValAttr(Type *Ty);
 
   /// This turns a sret type into the form used internally in Attribute.
   AttrBuilder &addStructRetAttr(Type *Ty);
 
   /// This turns a byref type into the form used internally in Attribute.
   AttrBuilder &addByRefAttr(Type *Ty);

llvm/lib/AsmParser/LLParser.cpp

while (true) {
     case lltok::kw_speculative_load_hardening:
       B.addAttribute(Attribute::SpeculativeLoadHardening);
       break;
     case lltok::kw_strictfp: B.addAttribute(Attribute::StrictFP); break;
     case lltok::kw_uwtable: B.addAttribute(Attribute::UWTable); break;
     case lltok::kw_willreturn: B.addAttribute(Attribute::WillReturn); break;
     case lltok::kw_writeonly: B.addAttribute(Attribute::WriteOnly); break;
     case lltok::kw_preallocated: {
-      Type *Ty;
-      if (parsePreallocated(Ty))
+      if (parseRequiredTypeAttr(B, lltok::kw_preallocated,
+                                Attribute::Preallocated))
         return true;
-      B.addPreallocatedAttr(Ty);
       break;
     }
 
     // error handling.
     case lltok::kw_inreg:
     case lltok::kw_signext:
     case lltok::kw_zeroext:
       HaveError |=
while (true) {
     }
     case lltok::kw_alignstack: {
       unsigned Alignment;
       if (parseOptionalStackAlignment(Alignment))
         return true;
       B.addStackAlignmentAttr(Alignment);
       continue;
     }
-    case lltok::kw_byval: {
-      Type *Ty;
-      if (parseRequiredTypeAttr(Ty, lltok::kw_byval))
+    case lltok::kw_byval:
+      if (parseRequiredTypeAttr(B, lltok::kw_byval, Attribute::ByVal))
         return true;
-      B.addByValAttr(Ty);
       continue;
-    }
-    case lltok::kw_sret: {
-      Type *Ty;
-      if (parseRequiredTypeAttr(Ty, lltok::kw_sret))
+    case lltok::kw_byref:
+      if (parseRequiredTypeAttr(B, lltok::kw_byref, Attribute::ByRef))
         return true;
-      B.addStructRetAttr(Ty);
       continue;
-    }
-    case lltok::kw_preallocated: {
-      Type *Ty;
-      if (parsePreallocated(Ty))
+    case lltok::kw_inalloca:
+      if (parseRequiredTypeAttr(B, lltok::kw_inalloca, Attribute::InAlloca))
         return true;
-      B.addPreallocatedAttr(Ty);
       continue;
-    }
-    case lltok::kw_inalloca: {
-      Type *Ty;
-      if (parseInalloca(Ty))
+    case lltok::kw_preallocated:
+      if (parseRequiredTypeAttr(B, lltok::kw_preallocated,
+                                Attribute::Preallocated))
+        return true;
+      continue;
+    case lltok::kw_sret:
+      if (parseRequiredTypeAttr(B, lltok::kw_sret, Attribute::StructRet))
         return true;
-      B.addInAllocaAttr(Ty);
       continue;
-    }
     case lltok::kw_dereferenceable: {
       uint64_t Bytes;
       if (parseOptionalDerefAttrBytes(lltok::kw_dereferenceable, Bytes))
         return true;
       B.addDereferenceableAttr(Bytes);
       continue;
     }
     case lltok::kw_dereferenceable_or_null: {
       uint64_t Bytes;
       if (parseOptionalDerefAttrBytes(lltok::kw_dereferenceable_or_null, Bytes))
         return true;
       B.addDereferenceableOrNullAttr(Bytes);
       continue;
     }
-    case lltok::kw_byref: {
-      Type *Ty;
-      if (parseByRef(Ty))
-        return true;
-      B.addByRefAttr(Ty);
-      continue;
-    }
     case lltok::kw_inreg:           B.addAttribute(Attribute::InReg); break;
     case lltok::kw_nest:            B.addAttribute(Attribute::Nest); break;
     case lltok::kw_noundef:
       B.addAttribute(Attribute::NoUndef);
       break;
     case lltok::kw_noalias:         B.addAttribute(Attribute::NoAlias); break;
     case lltok::kw_nocapture:       B.addAttribute(Attribute::NoCapture); break;
     case lltok::kw_nofree:          B.addAttribute(Attribute::NoFree); break;
return tokError("expected '...' at end of argument list for musttail call "
                     "in varargs function");
 
   Lex.Lex();  // Lex the ')'.
   return false;
 }
 
 /// parseRequiredTypeAttr
 ///   ::= attrname(<ty>)
-bool LLParser::parseRequiredTypeAttr(Type *&Result, lltok::Kind AttrName) {
-  Result = nullptr;
-  if (!EatIfPresent(AttrName))
+bool LLParser::parseRequiredTypeAttr(AttrBuilder &B, lltok::Kind AttrToken,
+                                     Attribute::AttrKind AttrKind) {
+  Type *Ty = nullptr;
+  if (!EatIfPresent(AttrToken))
     return true;
   if (!EatIfPresent(lltok::lparen))
     return error(Lex.getLoc(), "expected '('");
-  if (parseType(Result))
+  if (parseType(Ty))
     return true;
   if (!EatIfPresent(lltok::rparen))
     return error(Lex.getLoc(), "expected ')'");
-  return false;
-}
 
-/// parsePreallocated
-///   ::= preallocated(<ty>)
-bool LLParser::parsePreallocated(Type *&Result) {
-  return parseRequiredTypeAttr(Result, lltok::kw_preallocated);
-}
-
-/// parseInalloca
-///   ::= inalloca(<ty>)
-bool LLParser::parseInalloca(Type *&Result) {
-  return parseRequiredTypeAttr(Result, lltok::kw_inalloca);
-}
-
-/// parseByRef
-///   ::= byref(<type>)
-bool LLParser::parseByRef(Type *&Result) {
-  return parseRequiredTypeAttr(Result, lltok::kw_byref);
+  B.addTypeAttr(AttrKind, Ty);
+  return false;
 }
 
 /// parseOptionalOperandBundles
 ///    ::= /*empty*/
 ///    ::= '[' OperandBundle [, OperandBundle ]* ']'
 ///
 /// OperandBundle
 ///    ::= bundle-tag '(' ')'

llvm/lib/IR/Attributes.cpp

 void AttrBuilder::clear() {
   Attrs.reset();
   TargetDepAttrs.clear();
   Alignment.reset();
   StackAlignment.reset();
   DerefBytes = DerefOrNullBytes = 0;
   AllocSizeArgs = 0;
   VScaleRangeArgs = 0;
-  ByValType = nullptr;
-  StructRetType = nullptr;
-  ByRefType = nullptr;
-  PreallocatedType = nullptr;
+  TypeAttrs = {};
+}
+
+Optional<unsigned>
+AttrBuilder::kindToTypeIndex(Attribute::AttrKind Kind) const {
+  switch (Kind) {
+  case Attribute::ByVal:
+    return ByValTypeIndex;
+  case Attribute::ByRef:
+    return ByRefTypeIndex;
+  case Attribute::InAlloca:
+    return InAllocaTypeIndex;
+  case Attribute::Preallocated:
+    return PreallocatedTypeIndex;
+  case Attribute::StructRet:
+    return StructRetTypeIndex;
+  default:
+    return None;
+  }
 }
 
 AttrBuilder &AttrBuilder::addAttribute(Attribute Attr) {
   if (Attr.isStringAttribute()) {
     addAttribute(Attr.getKindAsString(), Attr.getValueAsString());
     return *this;
   }
 
   Attribute::AttrKind Kind = Attr.getKindAsEnum();
   Attrs[Kind] = true;
 
-  if (Kind == Attribute::Alignment)
+  if (Optional<unsigned> TypeIndex = kindToTypeIndex(Kind))
+    TypeAttrs[*TypeIndex] = Attr.getValueAsType();
+  else if (Kind == Attribute::Alignment)
     Alignment = Attr.getAlignment();
   else if (Kind == Attribute::StackAlignment)
     StackAlignment = Attr.getStackAlignment();
-  else if (Kind == Attribute::ByVal)
-    ByValType = Attr.getValueAsType();
-  else if (Kind == Attribute::StructRet)
-    StructRetType = Attr.getValueAsType();
-  else if (Kind == Attribute::ByRef)
-    ByRefType = Attr.getValueAsType();
-  else if (Kind == Attribute::Preallocated)
-    PreallocatedType = Attr.getValueAsType();
   else if (Kind == Attribute::Dereferenceable)
     DerefBytes = Attr.getDereferenceableBytes();
   else if (Kind == Attribute::DereferenceableOrNull)
     DerefOrNullBytes = Attr.getDereferenceableOrNullBytes();
   else if (Kind == Attribute::AllocSize)
     AllocSizeArgs = Attr.getValueAsInt();
   else if (Kind == Attribute::VScaleRange)
     VScaleRangeArgs = Attr.getValueAsInt();
-  else if (Kind == Attribute::InAlloca)
-    InAllocaType = Attr.getValueAsType();
 
   return *this;
 }
 
 AttrBuilder &AttrBuilder::addAttribute(StringRef A, StringRef V) {
   TargetDepAttrs[A] = V;
   return *this;
 }
 
 AttrBuilder &AttrBuilder::removeAttribute(Attribute::AttrKind Val) {
   assert((unsigned)Val < Attribute::EndAttrKinds && "Attribute out of range!");
   Attrs[Val] = false;
 
-  if (Val == Attribute::Alignment)
+  if (Optional<unsigned> TypeIndex = kindToTypeIndex(Val))
+    TypeAttrs[*TypeIndex] = nullptr;
+  else if (Val == Attribute::Alignment)
     Alignment.reset();
   else if (Val == Attribute::StackAlignment)
     StackAlignment.reset();
-  else if (Val == Attribute::ByVal)
-    ByValType = nullptr;
-  else if (Val == Attribute::StructRet)
-    StructRetType = nullptr;
-  else if (Val == Attribute::ByRef)
-    ByRefType = nullptr;
-  else if (Val == Attribute::Preallocated)
-    PreallocatedType = nullptr;
-  else if (Val == Attribute::InAlloca)
-    InAllocaType = nullptr;
   else if (Val == Attribute::Dereferenceable)
     DerefBytes = 0;
   else if (Val == Attribute::DereferenceableOrNull)
     DerefOrNullBytes = 0;
   else if (Val == Attribute::AllocSize)
     AllocSizeArgs = 0;
   else if (Val == Attribute::VScaleRange)
     VScaleRangeArgs = 0;
AttrBuilder &AttrBuilder::addVScaleRangeAttrFromRawRepr(uint64_t RawArgs) {
 
   Attrs[Attribute::VScaleRange] = true;
   // Reuse existing machinery to store this as a single 64-bit integer so we can
   // save a few bytes over using a pair<unsigned, unsigned>.
   VScaleRangeArgs = RawArgs;
   return *this;
 }
 
-AttrBuilder &AttrBuilder::addByValAttr(Type *Ty) {
-  Attrs[Attribute::ByVal] = true;
-  ByValType = Ty;
+AttrBuilder &AttrBuilder::addTypeAttr(Attribute::AttrKind Kind, Type *Ty) {
+  Optional<unsigned> TypeIndex = kindToTypeIndex(Kind);
+  assert(TypeIndex && "Not a type attribute");
+  Attrs[Kind] = true;
+  TypeAttrs[*TypeIndex] = Ty;
   return *this;
 }
 
+AttrBuilder &AttrBuilder::addByValAttr(Type *Ty) {
+  return addTypeAttr(Attribute::ByVal, Ty);
+}
+
 AttrBuilder &AttrBuilder::addStructRetAttr(Type *Ty) {
-  Attrs[Attribute::StructRet] = true;
-  StructRetType = Ty;
-  return *this;
+  return addTypeAttr(Attribute::StructRet, Ty);
 }
 
 AttrBuilder &AttrBuilder::addByRefAttr(Type *Ty) {
-  Attrs[Attribute::ByRef] = true;
-  ByRefType = Ty;
-  return *this;
+  return addTypeAttr(Attribute::ByRef, Ty);
 }
 
 AttrBuilder &AttrBuilder::addPreallocatedAttr(Type *Ty) {
-  Attrs[Attribute::Preallocated] = true;
-  PreallocatedType = Ty;
-  return *this;
+  return addTypeAttr(Attribute::Preallocated, Ty);
 }
 
 AttrBuilder &AttrBuilder::addInAllocaAttr(Type *Ty) {
-  Attrs[Attribute::InAlloca] = true;
-  InAllocaType = Ty;
-  return *this;
+  return addTypeAttr(Attribute::InAlloca, Ty);
 }
 
 AttrBuilder &AttrBuilder::merge(const AttrBuilder &B) {
   // FIXME: What if both have alignments, but they don't match?!
   if (!Alignment)
     Alignment = B.Alignment;
 
   if (!StackAlignment)
     StackAlignment = B.StackAlignment;
 
   if (!DerefBytes)
     DerefBytes = B.DerefBytes;
 
   if (!DerefOrNullBytes)
     DerefOrNullBytes = B.DerefOrNullBytes;
 
   if (!AllocSizeArgs)
     AllocSizeArgs = B.AllocSizeArgs;
 
-  if (!ByValType)
-    ByValType = B.ByValType;
-
-  if (!StructRetType)
-    StructRetType = B.StructRetType;
-
-  if (!ByRefType)
-    ByRefType = B.ByRefType;
-
-  if (!PreallocatedType)
-    PreallocatedType = B.PreallocatedType;
-
-  if (!InAllocaType)
-    InAllocaType = B.InAllocaType;
-
   if (!VScaleRangeArgs)
     VScaleRangeArgs = B.VScaleRangeArgs;
 
+  for (unsigned Index = 0; Index < NumTypeIndices; ++Index)
+    if (!TypeAttrs[Index])
+      TypeAttrs[Index] = B.TypeAttrs[Index];
+
   Attrs |= B.Attrs;
 
   for (const auto &I : B.td_attrs())
     TargetDepAttrs[I.first] = I.second;
 
   return *this;
 }
 
if (B.DerefBytes)
     DerefBytes = 0;
 
   if (B.DerefOrNullBytes)
     DerefOrNullBytes = 0;
 
   if (B.AllocSizeArgs)
     AllocSizeArgs = 0;
 
-  if (B.ByValType)
-    ByValType = nullptr;
-
-  if (B.StructRetType)
-    StructRetType = nullptr;
-
-  if (B.ByRefType)
-    ByRefType = nullptr;
-
-  if (B.PreallocatedType)
-    PreallocatedType = nullptr;
-
-  if (B.InAllocaType)
-    InAllocaType = nullptr;
-
   if (B.VScaleRangeArgs)
     VScaleRangeArgs = 0;
 
+  for (unsigned Index = 0; Index < NumTypeIndices; ++Index)
+    if (B.TypeAttrs[Index])
+      TypeAttrs[Index] = nullptr;
+
   Attrs &= ~B.Attrs;
 
   for (const auto &I : B.td_attrs())
     TargetDepAttrs.erase(I.first);
 
   return *this;
 }
 
bool AttrBuilder::operator==(const AttrBuilder &B) const {
   if (Attrs != B.Attrs)
     return false;
 
   for (const auto &TDA : TargetDepAttrs)
     if (B.TargetDepAttrs.find(TDA.first) == B.TargetDepAttrs.end())
       return false;
 
   return Alignment == B.Alignment && StackAlignment == B.StackAlignment &&
-         DerefBytes == B.DerefBytes && ByValType == B.ByValType &&
-         StructRetType == B.StructRetType && ByRefType == B.ByRefType &&
-         PreallocatedType == B.PreallocatedType &&
-         InAllocaType == B.InAllocaType &&
+         DerefBytes == B.DerefBytes && TypeAttrs == B.TypeAttrs &&
          VScaleRangeArgs == B.VScaleRangeArgs;
 }
 
 //===----------------------------------------------------------------------===//
 // AttributeFuncs Function Defintions
 //===----------------------------------------------------------------------===//
 
 /// Which attributes cannot be applied to a type.