Reland "[IR] Make AttributeSetNode public, avoid temporary AttributeList copies"

This re-lands r299875.

I introduced a bug in Clang code responsible for replacing K&R, no
prototype declarations with a real function definition with a prototype.
The bug was here:

       // Collect any return attributes from the call.
  -    if (oldAttrs.hasAttributes(llvm::AttributeList::ReturnIndex))
  -      newAttrs.push_back(llvm::AttributeList::get(newFn->getContext(),
  -                                                  oldAttrs.getRetAttributes()));
  +    newAttrs.push_back(oldAttrs.getRetAttributes());

Previously getRetAttributes() carried AttributeList::ReturnIndex in its
AttributeList. Now that we return the AttributeSetNode* directly, it no
longer carries that index, and we call this overload with a single node:
  AttributeList::get(LLVMContext&, ArrayRef<AttributeSetNode*>)

That aborted with an assertion on x86_32 targets. I added an explicit
triple to the test and added CHECKs to help find issues like this in the
future sooner.

llvm-svn: 299899

include/llvm/IR/AttributeSetNode.h

@@ -1,5 +1,4 @@
-//===-- AttributeSetNode.h - AttributeList Internal Node ---------*- C++
+//===-- AttributeSetNode.h - AttributeList Internal Node --------*- C++ -*-===//
-//-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -9,7 +8,8 @@
 //===----------------------------------------------------------------------===//
 ///
 /// \file
-/// \brief This file defines the node class used internally by AttributeList.
+/// \brief This file defines the class that represents a group of attributes
+/// that apply to one element: function, return type, or parameter.
 ///
 //===----------------------------------------------------------------------===//
 
@@ -64,6 +64,8 @@ public:
 
   void operator delete(void *p) { ::operator delete(p); }
 
+  static AttributeSetNode *get(LLVMContext &C, const AttrBuilder &B);
+
   static AttributeSetNode *get(LLVMContext &C, ArrayRef<Attribute> Attrs);
 
   static AttributeSetNode *get(AttributeList AS, unsigned Index) {

include/llvm/IR/Attributes.h

@@ -221,19 +221,26 @@ private:
   /// the empty attributes list.
   AttributeListImpl *pImpl = nullptr;
 
-  /// \brief The attributes for the specified index are returned.
+public:
-  AttributeSetNode *getAttributes(unsigned Index) const;
-
   /// \brief Create an AttributeList with the specified parameters in it.
   static AttributeList get(LLVMContext &C,
                            ArrayRef<std::pair<unsigned, Attribute>> Attrs);
   static AttributeList
   get(LLVMContext &C, ArrayRef<std::pair<unsigned, AttributeSetNode *>> Attrs);
 
+  /// \brief Create an AttributeList from a vector of AttributeSetNodes. The
+  /// index of each set is implied by its position in the array \p Attrs:
+  ///   0      : Return attributes
+  /// 1 to n-1 : Argument attributes
+  ///   n      : Function attributes
+  /// Any element that has no entries should be left null.
+  static AttributeList get(LLVMContext &C, ArrayRef<AttributeSetNode *> Attrs);
+
   static AttributeList
   getImpl(LLVMContext &C,
           ArrayRef<std::pair<unsigned, AttributeSetNode *>> Attrs);
 
+private:
   explicit AttributeList(AttributeListImpl *LI) : pImpl(LI) {}
 
 public:
@@ -272,6 +279,12 @@ public:
   AttributeList addAttributes(LLVMContext &C, unsigned Index,
                               AttributeList Attrs) const;
 
+  AttributeList addAttributes(LLVMContext &C, unsigned Index,
+                              AttributeSetNode *AS) const;
+
+  AttributeList addAttributes(LLVMContext &C, unsigned Index,
+                              const AttrBuilder &B) const;
+
   /// \brief Remove the specified attribute at the specified index from this
   /// attribute list. Because attribute lists are immutable, this returns the
   /// new list.
@@ -296,6 +309,11 @@ public:
   AttributeList removeAttributes(LLVMContext &C, unsigned Index,
                                  const AttrBuilder &Attrs) const;
 
+  /// \brief Remove all attributes at the specified index from this
+  /// attribute list. Because attribute lists are immutable, this returns the
+  /// new list.
+  AttributeList removeAttributes(LLVMContext &C, unsigned Index) const;
+
   /// \brief Add the dereferenceable attribute to the attribute set at the given
   /// index. Because attribute sets are immutable, this returns a new set.
   AttributeList addDereferenceableAttr(LLVMContext &C, unsigned Index,
@@ -321,13 +339,16 @@ public:
   LLVMContext &getContext() const;
 
   /// \brief The attributes for the specified index are returned.
-  AttributeList getParamAttributes(unsigned Index) const;
+  AttributeSetNode *getAttributes(unsigned Index) const;
+
+  /// \brief The attributes for the specified index are returned.
+  AttributeSetNode *getParamAttributes(unsigned Index) const;
 
   /// \brief The attributes for the ret value are returned.
-  AttributeList getRetAttributes() const;
+  AttributeSetNode *getRetAttributes() const;
 
   /// \brief The function attributes are returned.
-  AttributeList getFnAttributes() const;
+  AttributeSetNode *getFnAttributes() const;
 
   /// \brief Return true if the attribute exists at the given index.
   bool hasAttribute(unsigned Index, Attribute::AttrKind Kind) const;
@@ -462,6 +483,7 @@ public:
     addAttribute(A);
   }
   AttrBuilder(AttributeList AS, unsigned Idx);
+  AttrBuilder(AttributeSetNode *AS);
 
   void clear();
 
@@ -478,7 +500,7 @@ public:
   AttrBuilder &removeAttribute(Attribute::AttrKind Val);
 
   /// \brief Remove the attributes from the builder.
-  AttrBuilder &removeAttributes(AttributeList A, uint64_t Index);
+  AttrBuilder &removeAttributes(AttributeList A, uint64_t WithoutIndex);
 
   /// \brief Remove the target-dependent attribute to the builder.
   AttrBuilder &removeAttribute(StringRef A);

lib/AsmParser/LLParser.cpp

@@ -19,6 +19,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/AsmParser/SlotMapping.h"
 #include "llvm/IR/Argument.h"
+#include "llvm/IR/AttributeSetNode.h"
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CallingConv.h"
@@ -131,9 +132,8 @@ bool LLParser::ValidateEndOfModule() {
 
     if (Function *Fn = dyn_cast<Function>(V)) {
       AttributeList AS = Fn->getAttributes();
-      AttrBuilder FnAttrs(AS.getFnAttributes(), AttributeList::FunctionIndex);
+      AttrBuilder FnAttrs(AS.getFnAttributes());
-      AS = AS.removeAttributes(Context, AttributeList::FunctionIndex,
+      AS = AS.removeAttributes(Context, AttributeList::FunctionIndex);
-                               AS.getFnAttributes());
 
       FnAttrs.merge(B);
 
@@ -150,9 +150,8 @@ bool LLParser::ValidateEndOfModule() {
       Fn->setAttributes(AS);
     } else if (CallInst *CI = dyn_cast<CallInst>(V)) {
       AttributeList AS = CI->getAttributes();
-      AttrBuilder FnAttrs(AS.getFnAttributes(), AttributeList::FunctionIndex);
+      AttrBuilder FnAttrs(AS.getFnAttributes());
-      AS = AS.removeAttributes(Context, AttributeList::FunctionIndex,
+      AS = AS.removeAttributes(Context, AttributeList::FunctionIndex);
-                               AS.getFnAttributes());
       FnAttrs.merge(B);
       AS = AS.addAttributes(
           Context, AttributeList::FunctionIndex,
@@ -160,9 +159,8 @@ bool LLParser::ValidateEndOfModule() {
       CI->setAttributes(AS);
     } else if (InvokeInst *II = dyn_cast<InvokeInst>(V)) {
       AttributeList AS = II->getAttributes();
-      AttrBuilder FnAttrs(AS.getFnAttributes(), AttributeList::FunctionIndex);
+      AttrBuilder FnAttrs(AS.getFnAttributes());
-      AS = AS.removeAttributes(Context, AttributeList::FunctionIndex,
+      AS = AS.removeAttributes(Context, AttributeList::FunctionIndex);
-                               AS.getFnAttributes());
       FnAttrs.merge(B);
       AS = AS.addAttributes(
           Context, AttributeList::FunctionIndex,
@@ -2123,7 +2121,6 @@ bool LLParser::ParseParameterList(SmallVectorImpl<ParamInfo> &ArgList,
   if (ParseToken(lltok::lparen, "expected '(' in call"))
     return true;
 
-  unsigned AttrIndex = 1;
   while (Lex.getKind() != lltok::rparen) {
     // If this isn't the first argument, we need a comma.
     if (!ArgList.empty() &&
@@ -2158,7 +2155,7 @@ bool LLParser::ParseParameterList(SmallVectorImpl<ParamInfo> &ArgList,
         return true;
     }
     ArgList.push_back(ParamInfo(
-        ArgLoc, V, AttributeList::get(V->getContext(), AttrIndex++, ArgAttrs)));
+        ArgLoc, V, AttributeSetNode::get(V->getContext(), ArgAttrs)));
   }
 
   if (IsMustTailCall && InVarArgsFunc)
@@ -2263,9 +2260,8 @@ bool LLParser::ParseArgumentList(SmallVectorImpl<ArgInfo> &ArgList,
     if (!FunctionType::isValidArgumentType(ArgTy))
       return Error(TypeLoc, "invalid type for function argument");
 
-    unsigned AttrIndex = 1;
+    ArgList.emplace_back(TypeLoc, ArgTy,
-    ArgList.emplace_back(TypeLoc, ArgTy, AttributeList::get(ArgTy->getContext(),
+                         AttributeSetNode::get(ArgTy->getContext(), Attrs),
-                                                            AttrIndex++, Attrs),
                          std::move(Name));
 
     while (EatIfPresent(lltok::comma)) {
@@ -2292,10 +2288,9 @@ bool LLParser::ParseArgumentList(SmallVectorImpl<ArgInfo> &ArgList,
       if (!ArgTy->isFirstClassType())
         return Error(TypeLoc, "invalid type for function argument");
 
-      ArgList.emplace_back(
+      ArgList.emplace_back(TypeLoc, ArgTy,
-          TypeLoc, ArgTy,
+                           AttributeSetNode::get(ArgTy->getContext(), Attrs),
-          AttributeList::get(ArgTy->getContext(), AttrIndex++, Attrs),
+                           std::move(Name));
-          std::move(Name));
     }
   }
 
@@ -2319,7 +2314,7 @@ bool LLParser::ParseFunctionType(Type *&Result) {
   for (unsigned i = 0, e = ArgList.size(); i != e; ++i) {
     if (!ArgList[i].Name.empty())
       return Error(ArgList[i].Loc, "argument name invalid in function type");
-    if (ArgList[i].Attrs.hasAttributes(i + 1))
+    if (ArgList[i].Attrs)
       return Error(ArgList[i].Loc,
                    "argument attributes invalid in function type");
   }
@@ -4768,23 +4763,16 @@ bool LLParser::ParseFunctionHeader(Function *&Fn, bool isDefine) {
   // Okay, if we got here, the function is syntactically valid.  Convert types
   // and do semantic checks.
   std::vector<Type*> ParamTypeList;
-  SmallVector<AttributeList, 8> Attrs;
+  SmallVector<AttributeSetNode *, 8> Attrs;
 
-  if (RetAttrs.hasAttributes())
+  Attrs.push_back(AttributeSetNode::get(Context, RetAttrs));
-    Attrs.push_back(AttributeList::get(RetType->getContext(),
-                                       AttributeList::ReturnIndex, RetAttrs));
 
   for (unsigned i = 0, e = ArgList.size(); i != e; ++i) {
     ParamTypeList.push_back(ArgList[i].Ty);
-    if (ArgList[i].Attrs.hasAttributes(i + 1)) {
+    Attrs.push_back(ArgList[i].Attrs);
-      AttrBuilder B(ArgList[i].Attrs, i + 1);
-      Attrs.push_back(AttributeList::get(RetType->getContext(), i + 1, B));
-    }
   }
 
-  if (FuncAttrs.hasAttributes())
+  Attrs.push_back(AttributeSetNode::get(Context, FuncAttrs));
-    Attrs.push_back(AttributeList::get(
-        RetType->getContext(), AttributeList::FunctionIndex, FuncAttrs));
 
   AttributeList PAL = AttributeList::get(Context, Attrs);
 
@@ -5396,10 +5384,8 @@ bool LLParser::ParseInvoke(Instruction *&Inst, PerFunctionState &PFS) {
     return true;
 
   // Set up the Attribute for the function.
-  SmallVector<AttributeList, 8> Attrs;
+  SmallVector<AttributeSetNode *, 8> Attrs;
-  if (RetAttrs.hasAttributes())
+  Attrs.push_back(AttributeSetNode::get(Context, RetAttrs));
-    Attrs.push_back(AttributeList::get(RetType->getContext(),
-                                       AttributeList::ReturnIndex, RetAttrs));
 
   SmallVector<Value*, 8> Args;
 
@@ -5419,22 +5405,16 @@ bool LLParser::ParseInvoke(Instruction *&Inst, PerFunctionState &PFS) {
       return Error(ArgList[i].Loc, "argument is not of expected type '" +
                    getTypeString(ExpectedTy) + "'");
     Args.push_back(ArgList[i].V);
-    if (ArgList[i].Attrs.hasAttributes(i + 1)) {
+    Attrs.push_back(ArgList[i].Attrs);
-      AttrBuilder B(ArgList[i].Attrs, i + 1);
-      Attrs.push_back(AttributeList::get(RetType->getContext(), i + 1, B));
-    }
   }
 
   if (I != E)
     return Error(CallLoc, "not enough parameters specified for call");
 
-  if (FnAttrs.hasAttributes()) {
+  if (FnAttrs.hasAlignmentAttr())
-    if (FnAttrs.hasAlignmentAttr())
+    return Error(CallLoc, "invoke instructions may not have an alignment");
-      return Error(CallLoc, "invoke instructions may not have an alignment");
 
-    Attrs.push_back(AttributeList::get(RetType->getContext(),
+  Attrs.push_back(AttributeSetNode::get(Context, FnAttrs));
-                                       AttributeList::FunctionIndex, FnAttrs));
-  }
 
   // Finish off the Attribute and check them
   AttributeList PAL = AttributeList::get(Context, Attrs);
@@ -5998,10 +5978,8 @@ bool LLParser::ParseCall(Instruction *&Inst, PerFunctionState &PFS,
     return true;
 
   // Set up the Attribute for the function.
-  SmallVector<AttributeList, 8> Attrs;
+  SmallVector<AttributeSetNode *, 8> Attrs;
-  if (RetAttrs.hasAttributes())
+  Attrs.push_back(AttributeSetNode::get(Context, RetAttrs));
-    Attrs.push_back(AttributeList::get(RetType->getContext(),
-                                       AttributeList::ReturnIndex, RetAttrs));
 
   SmallVector<Value*, 8> Args;
 
@@ -6021,22 +5999,16 @@ bool LLParser::ParseCall(Instruction *&Inst, PerFunctionState &PFS,
       return Error(ArgList[i].Loc, "argument is not of expected type '" +
                    getTypeString(ExpectedTy) + "'");
     Args.push_back(ArgList[i].V);
-    if (ArgList[i].Attrs.hasAttributes(i + 1)) {
+    Attrs.push_back(ArgList[i].Attrs);
-      AttrBuilder B(ArgList[i].Attrs, i + 1);
-      Attrs.push_back(AttributeList::get(RetType->getContext(), i + 1, B));
-    }
   }
 
   if (I != E)
     return Error(CallLoc, "not enough parameters specified for call");
 
-  if (FnAttrs.hasAttributes()) {
+  if (FnAttrs.hasAlignmentAttr())
-    if (FnAttrs.hasAlignmentAttr())
+    return Error(CallLoc, "call instructions may not have an alignment");
-      return Error(CallLoc, "call instructions may not have an alignment");
 
-    Attrs.push_back(AttributeList::get(RetType->getContext(),
+  Attrs.push_back(AttributeSetNode::get(Context, FnAttrs));
-                                       AttributeList::FunctionIndex, FnAttrs));
-  }
 
   // Finish off the Attribute and check them
   AttributeList PAL = AttributeList::get(Context, Attrs);

lib/AsmParser/LLParser.h

@@ -397,8 +397,8 @@ namespace llvm {
     struct ParamInfo {
       LocTy Loc;
       Value *V;
-      AttributeList Attrs;
+      AttributeSetNode *Attrs;
-      ParamInfo(LocTy loc, Value *v, AttributeList attrs)
+      ParamInfo(LocTy loc, Value *v, AttributeSetNode *attrs)
           : Loc(loc), V(v), Attrs(attrs) {}
     };
     bool ParseParameterList(SmallVectorImpl<ParamInfo> &ArgList,
@@ -450,9 +450,9 @@ namespace llvm {
     struct ArgInfo {
       LocTy Loc;
       Type *Ty;
-      AttributeList Attrs;
+      AttributeSetNode *Attrs;
       std::string Name;
-      ArgInfo(LocTy L, Type *ty, AttributeList Attr, const std::string &N)
+      ArgInfo(LocTy L, Type *ty, AttributeSetNode *Attr, const std::string &N)
           : Loc(L), Ty(ty), Attrs(Attr), Name(N) {}
     };
     bool ParseArgumentList(SmallVectorImpl<ArgInfo> &ArgList, bool &isVarArg);

lib/IR/AsmWriter.cpp

@@ -21,6 +21,8 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/AssemblyAnnotationWriter.h"
+#include "llvm/IR/AttributeSetNode.h"
+#include "llvm/IR/Attributes.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/Constants.h"
@@ -604,7 +606,7 @@ private:
   unsigned mdnNext;
 
   /// asMap - The slot map for attribute sets.
-  DenseMap<AttributeList, unsigned> asMap;
+  DenseMap<AttributeSetNode *, unsigned> asMap;
   unsigned asNext;
 public:
   /// Construct from a module.
@@ -627,7 +629,7 @@ public:
   int getLocalSlot(const Value *V);
   int getGlobalSlot(const GlobalValue *V);
   int getMetadataSlot(const MDNode *N);
-  int getAttributeGroupSlot(AttributeList AS);
+  int getAttributeGroupSlot(AttributeSetNode *AS);
 
   /// If you'd like to deal with a function instead of just a module, use
   /// this method to get its data into the SlotTracker.
@@ -650,8 +652,8 @@ public:
   unsigned mdn_size() const { return mdnMap.size(); }
   bool mdn_empty() const { return mdnMap.empty(); }
 
-  /// AttributeList map iterators.
+  /// AttributeSetNode map iterators.
-  typedef DenseMap<AttributeList, unsigned>::iterator as_iterator;
+  typedef DenseMap<AttributeSetNode *, unsigned>::iterator as_iterator;
   as_iterator as_begin()   { return asMap.begin(); }
   as_iterator as_end()     { return asMap.end(); }
   unsigned as_size() const { return asMap.size(); }
@@ -671,8 +673,8 @@ private:
   /// CreateFunctionSlot - Insert the specified Value* into the slot table.
   void CreateFunctionSlot(const Value *V);
 
-  /// \brief Insert the specified AttributeList into the slot table.
+  /// \brief Insert the specified AttributeSetNode into the slot table.
-  void CreateAttributeSetSlot(AttributeList AS);
+  void CreateAttributeSetSlot(AttributeSetNode *AS);
 
   /// Add all of the module level global variables (and their initializers)
   /// and function declarations, but not the contents of those functions.
@@ -831,8 +833,8 @@ void SlotTracker::processModule() {
 
     // Add all the function attributes to the table.
     // FIXME: Add attributes of other objects?
-    AttributeList FnAttrs = F.getAttributes().getFnAttributes();
+    AttributeSetNode *FnAttrs = F.getAttributes().getFnAttributes();
-    if (FnAttrs.hasAttributes(AttributeList::FunctionIndex))
+    if (FnAttrs)
       CreateAttributeSetSlot(FnAttrs);
   }
 
@@ -869,13 +871,13 @@ void SlotTracker::processFunction() {
       // target may not be linked into the optimizer.
       if (const CallInst *CI = dyn_cast<CallInst>(&I)) {
         // Add all the call attributes to the table.
-        AttributeList Attrs = CI->getAttributes().getFnAttributes();
+        AttributeSetNode *Attrs = CI->getAttributes().getFnAttributes();
-        if (Attrs.hasAttributes(AttributeList::FunctionIndex))
+        if (Attrs)
           CreateAttributeSetSlot(Attrs);
       } else if (const InvokeInst *II = dyn_cast<InvokeInst>(&I)) {
         // Add all the call attributes to the table.
-        AttributeList Attrs = II->getAttributes().getFnAttributes();
+        AttributeSetNode *Attrs = II->getAttributes().getFnAttributes();
-        if (Attrs.hasAttributes(AttributeList::FunctionIndex))
+        if (Attrs)
           CreateAttributeSetSlot(Attrs);
       }
     }
@@ -961,11 +963,11 @@ int SlotTracker::getLocalSlot(const Value *V) {
   return FI == fMap.end() ? -1 : (int)FI->second;
 }
 
-int SlotTracker::getAttributeGroupSlot(AttributeList AS) {
+int SlotTracker::getAttributeGroupSlot(AttributeSetNode *AS) {
   // Check for uninitialized state and do lazy initialization.
   initialize();
 
-  // Find the AttributeList in the module map.
+  // Find the AttributeSetNode in the module map.
   as_iterator AI = asMap.find(AS);
   return AI == asMap.end() ? -1 : (int)AI->second;
 }
@@ -1015,9 +1017,8 @@ void SlotTracker::CreateMetadataSlot(const MDNode *N) {
       CreateMetadataSlot(Op);
 }
 
-void SlotTracker::CreateAttributeSetSlot(AttributeList AS) {
+void SlotTracker::CreateAttributeSetSlot(AttributeSetNode *AS) {
-  assert(AS.hasAttributes(AttributeList::FunctionIndex) &&
+  assert(AS && "Doesn't need a slot!");
-         "Doesn't need a slot!");
 
   as_iterator I = asMap.find(AS);
   if (I != asMap.end())
@@ -2606,17 +2607,10 @@ void AssemblyWriter::printFunction(const Function *F) {
 
   const AttributeList &Attrs = F->getAttributes();
   if (Attrs.hasAttributes(AttributeList::FunctionIndex)) {
-    AttributeList AS = Attrs.getFnAttributes();
+    AttributeSetNode *AS = Attrs.getFnAttributes();
     std::string AttrStr;
 
-    unsigned Idx = 0;
+    for (const Attribute &Attr : *AS) {
-    for (unsigned E = AS.getNumSlots(); Idx != E; ++Idx)
-      if (AS.getSlotIndex(Idx) == AttributeList::FunctionIndex)
-        break;
-
-    for (AttributeList::iterator I = AS.begin(Idx), E = AS.end(Idx); I != E;
-         ++I) {
-      Attribute Attr = *I;
       if (!Attr.isStringAttribute()) {
         if (!AttrStr.empty()) AttrStr += ' ';
         AttrStr += Attr.getAsString();
@@ -3256,7 +3250,7 @@ void AssemblyWriter::printMDNodeBody(const MDNode *Node) {
 }
 
 void AssemblyWriter::writeAllAttributeGroups() {
-  std::vector<std::pair<AttributeList, unsigned>> asVec;
+  std::vector<std::pair<AttributeSetNode *, unsigned>> asVec;
   asVec.resize(Machine.as_size());
 
   for (SlotTracker::as_iterator I = Machine.as_begin(), E = Machine.as_end();
@@ -3265,7 +3259,7 @@ void AssemblyWriter::writeAllAttributeGroups() {
 
   for (const auto &I : asVec)
     Out << "attributes #" << I.second << " = { "
-        << I.first.getAsString(AttributeList::FunctionIndex, true) << " }\n";
+        << I.first->getAsString(true) << " }\n";
 }
 
 void AssemblyWriter::printUseListOrder(const UseListOrder &Order) {

lib/IR/AttributeImpl.h

@@ -16,10 +16,10 @@
 #ifndef LLVM_LIB_IR_ATTRIBUTEIMPL_H
 #define LLVM_LIB_IR_ATTRIBUTEIMPL_H
 
-#include "AttributeSetNode.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/IR/AttributeSetNode.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/Support/TrailingObjects.h"
 #include <algorithm>

lib/IR/Attributes.cpp

@@ -14,7 +14,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "AttributeImpl.h"
-#include "AttributeSetNode.h"
 #include "LLVMContextImpl.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FoldingSet.h"
@@ -24,6 +23,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/IR/AttributeSetNode.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/LLVMContext.h"
@@ -527,6 +527,48 @@ AttributeSetNode *AttributeSetNode::get(LLVMContext &C,
   return PA;
 }
 
+AttributeSetNode *AttributeSetNode::get(LLVMContext &C, const AttrBuilder &B) {
+  // Add target-independent attributes.
+  SmallVector<Attribute, 8> Attrs;
+  for (Attribute::AttrKind Kind = Attribute::None;
+       Kind != Attribute::EndAttrKinds; Kind = Attribute::AttrKind(Kind + 1)) {
+    if (!B.contains(Kind))
+      continue;
+
+    Attribute Attr;
+    switch (Kind) {
+    case Attribute::Alignment:
+      Attr = Attribute::getWithAlignment(C, B.getAlignment());
+      break;
+    case Attribute::StackAlignment:
+      Attr = Attribute::getWithStackAlignment(C, B.getStackAlignment());
+      break;
+    case Attribute::Dereferenceable:
+      Attr = Attribute::getWithDereferenceableBytes(
+          C, B.getDereferenceableBytes());
+      break;
+    case Attribute::DereferenceableOrNull:
+      Attr = Attribute::getWithDereferenceableOrNullBytes(
+          C, B.getDereferenceableOrNullBytes());
+      break;
+    case Attribute::AllocSize: {
+      auto A = B.getAllocSizeArgs();
+      Attr = Attribute::getWithAllocSizeArgs(C, A.first, A.second);
+      break;
+    }
+    default:
+      Attr = Attribute::get(C, Kind);
+    }
+    Attrs.push_back(Attr);
+  }
+
+  // Add target-dependent (string) attributes.
+  for (const auto &TDA : B.td_attrs())
+    Attrs.emplace_back(Attribute::get(C, TDA.first, TDA.second));
+
+  return get(C, Attrs);
+}
+
 bool AttributeSetNode::hasAttribute(StringRef Kind) const {
   for (Attribute I : *this)
     if (I.hasAttribute(Kind))
@@ -612,6 +654,19 @@ LLVM_DUMP_METHOD void AttributeListImpl::dump() const {
 
 AttributeList AttributeList::getImpl(
     LLVMContext &C, ArrayRef<std::pair<unsigned, AttributeSetNode *>> Attrs) {
+  assert(!Attrs.empty() && "creating pointless AttributeList");
+#ifndef NDEBUG
+  unsigned LastIndex = 0;
+  bool IsFirst = true;
+  for (const auto &AttrPair : Attrs) {
+    assert((IsFirst || LastIndex < AttrPair.first) &&
+           "unsorted or duplicate AttributeList indices");
+    assert(AttrPair.second && "pointless AttributeList slot");
+    LastIndex = AttrPair.first;
+    IsFirst = false;
+  }
+#endif
+
   LLVMContextImpl *pImpl = C.pImpl;
   FoldingSetNodeID ID;
   AttributeListImpl::Profile(ID, Attrs);
@@ -680,50 +735,32 @@ AttributeList::get(LLVMContext &C,
   return getImpl(C, Attrs);
 }
 
+AttributeList AttributeList::get(LLVMContext &C, ArrayRef<AttributeSetNode*> Attrs) {
+  assert(Attrs.size() >= 2 &&
+         "should always have function and return attr slots");
+  SmallVector<std::pair<unsigned, AttributeSetNode *>, 8> AttrPairs;
+  size_t Index = 0;
+  for (AttributeSetNode *AS : Attrs) {
+    if (AS) {
+      // If this is the last AttributeSetNode, it's for the function.
+      if (Index == Attrs.size() - 1)
+        Index = AttributeList::FunctionIndex;
+      AttrPairs.emplace_back(Index, AS);
+    }
+    ++Index;
+  }
+  if (AttrPairs.empty())
+    return AttributeList();
+  return getImpl(C, AttrPairs);
+}
+
 AttributeList AttributeList::get(LLVMContext &C, unsigned Index,
                                  const AttrBuilder &B) {
   if (!B.hasAttributes())
     return AttributeList();
-
+  AttributeSetNode *ASN = AttributeSetNode::get(C, B);
-  // Add target-independent attributes.
+  std::pair<unsigned, AttributeSetNode *> Arr[1] = {{Index, ASN}};
-  SmallVector<std::pair<unsigned, Attribute>, 8> Attrs;
+  return getImpl(C, Arr);
-  for (Attribute::AttrKind Kind = Attribute::None;
-       Kind != Attribute::EndAttrKinds; Kind = Attribute::AttrKind(Kind + 1)) {
-    if (!B.contains(Kind))
-      continue;
-
-    Attribute Attr;
-    switch (Kind) {
-    case Attribute::Alignment:
-      Attr = Attribute::getWithAlignment(C, B.getAlignment());
-      break;
-    case Attribute::StackAlignment:
-      Attr = Attribute::getWithStackAlignment(C, B.getStackAlignment());
-      break;
-    case Attribute::Dereferenceable:
-      Attr = Attribute::getWithDereferenceableBytes(
-          C, B.getDereferenceableBytes());
-      break;
-    case Attribute::DereferenceableOrNull:
-      Attr = Attribute::getWithDereferenceableOrNullBytes(
-          C, B.getDereferenceableOrNullBytes());
-      break;
-    case Attribute::AllocSize: {
-      auto A = B.getAllocSizeArgs();
-      Attr = Attribute::getWithAllocSizeArgs(C, A.first, A.second);
-      break;
-    }
-    default:
-      Attr = Attribute::get(C, Kind);
-    }
-    Attrs.emplace_back(Index, Attr);
-  }
-
-  // Add target-dependent (string) attributes.
-  for (const auto &TDA : B.td_attrs())
-    Attrs.emplace_back(Index, Attribute::get(C, TDA.first, TDA.second));
-
-  return get(C, Attrs);
 }
 
 AttributeList AttributeList::get(LLVMContext &C, unsigned Index,
@@ -791,31 +828,31 @@ AttributeList AttributeList::addAttribute(LLVMContext &C, unsigned Index,
 AttributeList AttributeList::addAttribute(LLVMContext &C,
                                           ArrayRef<unsigned> Indices,
                                           Attribute A) const {
-  unsigned I = 0, E = pImpl ? pImpl->getNumSlots() : 0;
+  assert(std::is_sorted(Indices.begin(), Indices.end()));
-  auto IdxI = Indices.begin(), IdxE = Indices.end();
-  SmallVector<AttributeList, 4> AttrSet;
 
-  while (I != E && IdxI != IdxE) {
+  unsigned I = 0, E = pImpl ? pImpl->getNumSlots() : 0;
-    if (getSlotIndex(I) < *IdxI)
+  SmallVector<IndexAttrPair, 4> AttrVec;
-      AttrSet.emplace_back(getSlotAttributes(I++));
+  for (unsigned Index : Indices) {
-    else if (getSlotIndex(I) > *IdxI)
+    // Add all attribute slots before the current index.
-      AttrSet.emplace_back(AttributeList::get(C, std::make_pair(*IdxI++, A)));
+    for (; I < E && getSlotIndex(I) < Index; ++I)
-    else {
+      AttrVec.emplace_back(getSlotIndex(I), pImpl->getSlotNode(I));
-      AttrBuilder B(getSlotAttributes(I), *IdxI);
+
-      B.addAttribute(A);
+    // Add the attribute at this index. If we already have attributes at this
-      AttrSet.emplace_back(AttributeList::get(C, *IdxI, B));
+    // index, merge them into a new set.
+    AttrBuilder B;
+    if (I < E && getSlotIndex(I) == Index) {
+      B.merge(AttrBuilder(pImpl->getSlotNode(I)));
       ++I;
-      ++IdxI;
     }
+    B.addAttribute(A);
+    AttrVec.emplace_back(Index, AttributeSetNode::get(C, B));
   }
 
-  while (I != E)
+  // Add remaining attributes.
-    AttrSet.emplace_back(getSlotAttributes(I++));
+  for (; I < E; ++I)
+    AttrVec.emplace_back(getSlotIndex(I), pImpl->getSlotNode(I));
 
-  while (IdxI != IdxE)
+  return get(C, AttrVec);
-    AttrSet.emplace_back(AttributeList::get(C, std::make_pair(*IdxI++, A)));
-
-  return get(C, AttrSet);
 }
 
 AttributeList AttributeList::addAttributes(LLVMContext &C, unsigned Index,
@@ -823,51 +860,58 @@ AttributeList AttributeList::addAttributes(LLVMContext &C, unsigned Index,
   if (!pImpl) return Attrs;
   if (!Attrs.pImpl) return *this;
 
+  return addAttributes(C, Index, Attrs.getAttributes(Index));
+}
+
+AttributeList AttributeList::addAttributes(LLVMContext &C, unsigned Index,
+                                           AttributeSetNode *AS) const {
+  if (!AS)
+    return *this;
+
 #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 = getParamAlignment(Index);
-  unsigned NewAlign = Attrs.getParamAlignment(Index);
+  unsigned NewAlign = AS->getAlignment();
   assert((!OldAlign || !NewAlign || OldAlign == NewAlign) &&
          "Attempt to change alignment!");
 #endif
 
-  // Add the attribute slots before the one we're trying to add.
+  SmallVector<std::pair<unsigned, AttributeSetNode *>, 4> AttrSet;
-  SmallVector<AttributeList, 4> AttrSet;
   uint64_t NumAttrs = pImpl->getNumSlots();
-  AttributeList AS;
+  unsigned I;
-  uint64_t LastIndex = 0;
+
-  for (unsigned I = 0, E = NumAttrs; I != E; ++I) {
+  // Add all the attribute slots before the one we need to merge.
-    if (getSlotIndex(I) >= Index) {
+  for (I = 0; I < NumAttrs; ++I) {
-      if (getSlotIndex(I) == Index) AS = getSlotAttributes(LastIndex++);
+    if (getSlotIndex(I) >= Index)
       break;
-    }
+    AttrSet.emplace_back(getSlotIndex(I), pImpl->getSlotNode(I));
-    LastIndex = I + 1;
-    AttrSet.push_back(getSlotAttributes(I));
   }
 
-  // Now add the attribute into the correct slot. There may already be an
+  if (I < NumAttrs && getSlotIndex(I) == Index) {
-  // AttributeList there.
+    // We need to merge two AttributeSetNodes.
-  AttrBuilder B(AS, Index);
+    AttributeSetNode *Merged = AttributeSetNode::get(
+        C, AttrBuilder(pImpl->getSlotNode(I)).merge(AttrBuilder(AS)));
+    AttrSet.emplace_back(Index, Merged);
+    ++I;
+  } else {
+    // Otherwise, there were no attributes at this position in the original
+    // list. Add the set as is.
+    AttrSet.emplace_back(Index, AS);
+  }
 
-  for (unsigned I = 0, E = Attrs.pImpl->getNumSlots(); I != E; ++I)
+  // Add the remaining entries.
-    if (Attrs.getSlotIndex(I) == Index) {
+  for (; I < NumAttrs; ++I)
-      for (AttributeListImpl::iterator II = Attrs.pImpl->begin(I),
+    AttrSet.emplace_back(getSlotIndex(I), pImpl->getSlotNode(I));
-                                       IE = Attrs.pImpl->end(I);
-           II != IE; ++II)
-        B.addAttribute(*II);
-      break;
-    }
-
-  AttrSet.push_back(AttributeList::get(C, Index, B));
-
-  // Add the remaining attribute slots.
-  for (unsigned I = LastIndex, E = NumAttrs; I < E; ++I)
-    AttrSet.push_back(getSlotAttributes(I));
 
   return get(C, AttrSet);
 }
 
+AttributeList AttributeList::addAttributes(LLVMContext &C, unsigned Index,
+                                           const AttrBuilder &B) const {
+  return get(C, Index, AttributeSetNode::get(C, B));
+}
+
 AttributeList AttributeList::removeAttribute(LLVMContext &C, unsigned Index,
                                              Attribute::AttrKind Kind) const {
   if (!hasAttribute(Index, Kind)) return *this;
@@ -961,6 +1005,20 @@ AttributeList AttributeList::removeAttributes(LLVMContext &C, unsigned Index,
   return get(C, AttrSet);
 }
 
+AttributeList AttributeList::removeAttributes(LLVMContext &C,
+                                              unsigned WithoutIndex) const {
+  if (!pImpl)
+    return AttributeList();
+
+  SmallVector<std::pair<unsigned, AttributeSetNode *>, 4> AttrSet;
+  for (unsigned I = 0, E = pImpl->getNumSlots(); I != E; ++I) {
+    unsigned Index = getSlotIndex(I);
+    if (Index != WithoutIndex)
+      AttrSet.push_back({Index, pImpl->getSlotNode(I)});
+  }
+  return get(C, AttrSet);
+}
+
 AttributeList AttributeList::addDereferenceableAttr(LLVMContext &C,
                                                     unsigned Index,
                                                     uint64_t Bytes) const {
@@ -992,32 +1050,16 @@ AttributeList::addAllocSizeAttr(LLVMContext &C, unsigned Index,
 
 LLVMContext &AttributeList::getContext() const { return pImpl->getContext(); }
 
-AttributeList AttributeList::getParamAttributes(unsigned Index) const {
+AttributeSetNode *AttributeList::getParamAttributes(unsigned Index) const {
-  return pImpl && hasAttributes(Index)
+  return getAttributes(Index);
-             ? AttributeList::get(
-                   pImpl->getContext(),
-                   ArrayRef<std::pair<unsigned, AttributeSetNode *>>(
-                       std::make_pair(Index, getAttributes(Index))))
-             : AttributeList();
 }
 
-AttributeList AttributeList::getRetAttributes() const {
+AttributeSetNode *AttributeList::getRetAttributes() const {
-  return pImpl && hasAttributes(ReturnIndex)
+  return getAttributes(ReturnIndex);
-             ? AttributeList::get(
-                   pImpl->getContext(),
-                   ArrayRef<std::pair<unsigned, AttributeSetNode *>>(
-                       std::make_pair(ReturnIndex, getAttributes(ReturnIndex))))
-             : AttributeList();
 }
 
-AttributeList AttributeList::getFnAttributes() const {
+AttributeSetNode *AttributeList::getFnAttributes() const {
-  return pImpl && hasAttributes(FunctionIndex)
+  return getAttributes(FunctionIndex);
-             ? AttributeList::get(
-                   pImpl->getContext(),
-                   ArrayRef<std::pair<unsigned, AttributeSetNode *>>(
-                       std::make_pair(FunctionIndex,
-                                      getAttributes(FunctionIndex))))
-             : AttributeList();
 }
 
 bool AttributeList::hasAttribute(unsigned Index,
@@ -1181,6 +1223,13 @@ AttrBuilder::AttrBuilder(AttributeList AS, unsigned Index) {
   }
 }
 
+AttrBuilder::AttrBuilder(AttributeSetNode *AS) {
+  if (AS) {
+    for (const Attribute &A : *AS)
+      addAttribute(A);
+  }
+}
+
 void AttrBuilder::clear() {
   Attrs.reset();
   TargetDepAttrs.clear();

lib/IR/Core.cpp

@@ -16,7 +16,7 @@
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/IR/Attributes.h"
-#include "AttributeSetNode.h"
+#include "llvm/IR/AttributeSetNode.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"

lib/Transforms/IPO/ArgumentPromotion.cpp

@@ -42,6 +42,7 @@
 #include "llvm/Analysis/LazyCallGraph.h"
 #include "llvm/Analysis/Loads.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/IR/AttributeSetNode.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
@@ -102,13 +103,11 @@ doPromotion(Function *F, SmallPtrSetImpl<Argument *> &ArgsToPromote,
   // Attribute - Keep track of the parameter attributes for the arguments
   // that we are *not* promoting. For the ones that we do promote, the parameter
   // attributes are lost
-  SmallVector<AttributeList, 8> AttributesVec;
+  SmallVector<AttributeSetNode *, 8> AttributesVec;
   const AttributeList &PAL = F->getAttributes();
 
   // Add any return attributes.
-  if (PAL.hasAttributes(AttributeList::ReturnIndex))
+  AttributesVec.push_back(PAL.getRetAttributes());
-    AttributesVec.push_back(
-        AttributeList::get(F->getContext(), PAL.getRetAttributes()));
 
   // First, determine the new argument list
   unsigned ArgIndex = 1;
@@ -119,16 +118,12 @@ doPromotion(Function *F, SmallPtrSetImpl<Argument *> &ArgsToPromote,
       Type *AgTy = cast<PointerType>(I->getType())->getElementType();
       StructType *STy = cast<StructType>(AgTy);
       Params.insert(Params.end(), STy->element_begin(), STy->element_end());
+      AttributesVec.insert(AttributesVec.end(), STy->getNumElements(), nullptr);
       ++NumByValArgsPromoted;
     } else if (!ArgsToPromote.count(&*I)) {
       // Unchanged argument
       Params.push_back(I->getType());
-      AttributeList attrs = PAL.getParamAttributes(ArgIndex);
+      AttributesVec.push_back(PAL.getParamAttributes(ArgIndex));
-      if (attrs.hasAttributes(ArgIndex)) {
-        AttrBuilder B(attrs, ArgIndex);
-        AttributesVec.push_back(
-            AttributeList::get(F->getContext(), Params.size(), B));
-      }
     } else if (I->use_empty()) {
       // Dead argument (which are always marked as promotable)
       ++NumArgumentsDead;
@@ -173,6 +168,7 @@ doPromotion(Function *F, SmallPtrSetImpl<Argument *> &ArgsToPromote,
         Params.push_back(GetElementPtrInst::getIndexedType(
             cast<PointerType>(I->getType()->getScalarType())->getElementType(),
             ArgIndex.second));
+        AttributesVec.push_back(nullptr);
         assert(Params.back());
       }
 
@@ -184,9 +180,7 @@ doPromotion(Function *F, SmallPtrSetImpl<Argument *> &ArgsToPromote,
   }
 
   // Add any function attributes.
-  if (PAL.hasAttributes(AttributeList::FunctionIndex))
+  AttributesVec.push_back(PAL.getFnAttributes());
-    AttributesVec.push_back(
-        AttributeList::get(FTy->getContext(), PAL.getFnAttributes()));
 
   Type *RetTy = FTy->getReturnType();
 
@@ -223,9 +217,7 @@ doPromotion(Function *F, SmallPtrSetImpl<Argument *> &ArgsToPromote,
     const AttributeList &CallPAL = CS.getAttributes();
 
     // Add any return attributes.
-    if (CallPAL.hasAttributes(AttributeList::ReturnIndex))
+    AttributesVec.push_back(CallPAL.getRetAttributes());
-      AttributesVec.push_back(
-          AttributeList::get(F->getContext(), CallPAL.getRetAttributes()));
 
     // Loop over the operands, inserting GEP and loads in the caller as
     // appropriate.
@@ -235,12 +227,7 @@ doPromotion(Function *F, SmallPtrSetImpl<Argument *> &ArgsToPromote,
          ++I, ++AI, ++ArgIndex)
       if (!ArgsToPromote.count(&*I) && !ByValArgsToTransform.count(&*I)) {
         Args.push_back(*AI); // Unmodified argument
-
+        AttributesVec.push_back(CallPAL.getAttributes(ArgIndex));
-        if (CallPAL.hasAttributes(ArgIndex)) {
-          AttrBuilder B(CallPAL, ArgIndex);
-          AttributesVec.push_back(
-              AttributeList::get(F->getContext(), Args.size(), B));
-        }
       } else if (ByValArgsToTransform.count(&*I)) {
         // Emit a GEP and load for each element of the struct.
         Type *AgTy = cast<PointerType>(I->getType())->getElementType();
@@ -253,6 +240,7 @@ doPromotion(Function *F, SmallPtrSetImpl<Argument *> &ArgsToPromote,
               STy, *AI, Idxs, (*AI)->getName() + "." + Twine(i), Call);
           // TODO: Tell AA about the new values?
           Args.push_back(new LoadInst(Idx, Idx->getName() + ".val", Call));
+          AttributesVec.push_back(nullptr);
         }
       } else if (!I->use_empty()) {
         // Non-dead argument: insert GEPs and loads as appropriate.
@@ -295,23 +283,18 @@ doPromotion(Function *F, SmallPtrSetImpl<Argument *> &ArgsToPromote,
           newLoad->setAAMetadata(AAInfo);
 
           Args.push_back(newLoad);
+          AttributesVec.push_back(nullptr);
         }
       }
 
     // Push any varargs arguments on the list.
     for (; AI != CS.arg_end(); ++AI, ++ArgIndex) {
       Args.push_back(*AI);
-      if (CallPAL.hasAttributes(ArgIndex)) {
+      AttributesVec.push_back(CallPAL.getAttributes(ArgIndex));
-        AttrBuilder B(CallPAL, ArgIndex);
-        AttributesVec.push_back(
-            AttributeList::get(F->getContext(), Args.size(), B));
-      }
     }
 
     // Add any function attributes.
-    if (CallPAL.hasAttributes(AttributeList::FunctionIndex))
+    AttributesVec.push_back(CallPAL.getFnAttributes());
-      AttributesVec.push_back(
-          AttributeList::get(Call->getContext(), CallPAL.getFnAttributes()));
 
     SmallVector<OperandBundleDef, 1> OpBundles;
     CS.getOperandBundlesAsDefs(OpBundles);

lib/Transforms/IPO/DeadArgumentElimination.cpp

@@ -21,6 +21,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/IR/AttributeSetNode.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/Constant.h"
@@ -172,8 +173,9 @@ bool DeadArgumentEliminationPass::DeleteDeadVarargs(Function &Fn) {
       for (unsigned i = 0; PAL.getSlotIndex(i) <= NumArgs; ++i)
         AttributesVec.push_back(PAL.getSlotAttributes(i));
       if (PAL.hasAttributes(AttributeList::FunctionIndex))
-        AttributesVec.push_back(
+        AttributesVec.push_back(AttributeList::get(Fn.getContext(),
-            AttributeList::get(Fn.getContext(), PAL.getFnAttributes()));
+                                                   AttributeList::FunctionIndex,
+                                                   PAL.getFnAttributes()));
       PAL = AttributeList::get(Fn.getContext(), AttributesVec);
     }
 
@@ -684,9 +686,13 @@ bool DeadArgumentEliminationPass::RemoveDeadStuffFromFunction(Function *F) {
   bool HasLiveReturnedArg = false;
 
   // Set up to build a new list of parameter attributes.
-  SmallVector<AttributeList, 8> AttributesVec;
+  SmallVector<AttributeSetNode *, 8> AttributesVec;
   const AttributeList &PAL = F->getAttributes();
 
+  // Reserve an empty slot for the return value attributes, which we will
+  // compute last.
+  AttributesVec.push_back(nullptr);
+
   // Remember which arguments are still alive.
   SmallVector<bool, 10> ArgAlive(FTy->getNumParams(), false);
   // Construct the new parameter list from non-dead arguments. Also construct
@@ -699,16 +705,8 @@ bool DeadArgumentEliminationPass::RemoveDeadStuffFromFunction(Function *F) {
     if (LiveValues.erase(Arg)) {
       Params.push_back(I->getType());
       ArgAlive[i] = true;
-
+      AttributesVec.push_back(PAL.getParamAttributes(i + 1));
-      // Get the original parameter attributes (skipping the first one, that is
+      HasLiveReturnedArg |= PAL.hasAttribute(i + 1, Attribute::Returned);
-      // for the return value.
-      if (PAL.hasAttributes(i + 1)) {
-        AttrBuilder B(PAL, i + 1);
-        if (B.contains(Attribute::Returned))
-          HasLiveReturnedArg = true;
-        AttributesVec.push_back(
-            AttributeList::get(F->getContext(), Params.size(), B));
-      }
     } else {
       ++NumArgumentsEliminated;
       DEBUG(dbgs() << "DeadArgumentEliminationPass - Removing argument " << i
@@ -782,29 +780,25 @@ bool DeadArgumentEliminationPass::RemoveDeadStuffFromFunction(Function *F) {
   assert(NRetTy && "No new return type found?");
 
   // The existing function return attributes.
-  AttributeList RAttrs = PAL.getRetAttributes();
+  AttrBuilder RAttrs(PAL.getRetAttributes());
 
   // Remove any incompatible attributes, but only if we removed all return
   // values. Otherwise, ensure that we don't have any conflicting attributes
   // here. Currently, this should not be possible, but special handling might be
   // required when new return value attributes are added.
   if (NRetTy->isVoidTy())
-    RAttrs = RAttrs.removeAttributes(NRetTy->getContext(),
+    RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy));
-                                     AttributeList::ReturnIndex,
-                                     AttributeFuncs::typeIncompatible(NRetTy));
   else
-    assert(!AttrBuilder(RAttrs, AttributeList::ReturnIndex)
+    assert(!RAttrs.overlaps(AttributeFuncs::typeIncompatible(NRetTy)) &&
-                .overlaps(AttributeFuncs::typeIncompatible(NRetTy)) &&
            "Return attributes no longer compatible?");
 
-  if (RAttrs.hasAttributes(AttributeList::ReturnIndex))
+  AttributesVec[0] = AttributeSetNode::get(F->getContext(), RAttrs);
-    AttributesVec.push_back(AttributeList::get(NRetTy->getContext(), RAttrs));
 
-  if (PAL.hasAttributes(AttributeList::FunctionIndex))
+  // Transfer the function attributes, if any.
-    AttributesVec.push_back(
+  AttributesVec.push_back(PAL.getFnAttributes());
-        AttributeList::get(F->getContext(), PAL.getFnAttributes()));
 
   // Reconstruct the AttributesList based on the vector we constructed.
+  assert(AttributesVec.size() == Params.size() + 2);
   AttributeList NewPAL = AttributeList::get(F->getContext(), AttributesVec);
 
   // Create the new function type based on the recomputed parameters.
@@ -835,15 +829,11 @@ bool DeadArgumentEliminationPass::RemoveDeadStuffFromFunction(Function *F) {
     AttributesVec.clear();
     const AttributeList &CallPAL = CS.getAttributes();
 
-    // The call return attributes.
+    // Adjust the call return attributes in case the function was changed to
-    AttributeList RAttrs = CallPAL.getRetAttributes();
+    // return void.
-
+    AttrBuilder RAttrs(CallPAL.getRetAttributes());
-    // Adjust in case the function was changed to return void.
+    RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy));
-    RAttrs = RAttrs.removeAttributes(
+    AttributesVec.push_back(AttributeSetNode::get(F->getContext(), RAttrs));
-        NRetTy->getContext(), AttributeList::ReturnIndex,
-        AttributeFuncs::typeIncompatible(NF->getReturnType()));
-    if (RAttrs.hasAttributes(AttributeList::ReturnIndex))
-      AttributesVec.push_back(AttributeList::get(NF->getContext(), RAttrs));
 
     // Declare these outside of the loops, so we can reuse them for the second
     // loop, which loops the varargs.
@@ -855,33 +845,30 @@ bool DeadArgumentEliminationPass::RemoveDeadStuffFromFunction(Function *F) {
       if (ArgAlive[i]) {
         Args.push_back(*I);
         // Get original parameter attributes, but skip return attributes.
-        if (CallPAL.hasAttributes(i + 1)) {
+        AttributeSetNode *Attrs = CallPAL.getParamAttributes(i + 1);
-          AttrBuilder B(CallPAL, i + 1);
+        if (NRetTy != RetTy && Attrs &&
+            Attrs->hasAttribute(Attribute::Returned)) {
           // If the return type has changed, then get rid of 'returned' on the
           // call site. The alternative is to make all 'returned' attributes on
           // call sites keep the return value alive just like 'returned'
-          // attributes on function declaration but it's less clearly a win
+          // attributes on function declaration but it's less clearly a win and
-          // and this is not an expected case anyway
+          // this is not an expected case anyway
-          if (NRetTy != RetTy && B.contains(Attribute::Returned))
+          AttributesVec.push_back(AttributeSetNode::get(
-            B.removeAttribute(Attribute::Returned);
+              F->getContext(),
-          AttributesVec.push_back(
+              AttrBuilder(Attrs).removeAttribute(Attribute::Returned)));
-              AttributeList::get(F->getContext(), Args.size(), B));
+        } else {
+          // Otherwise, use the original attributes.
+          AttributesVec.push_back(Attrs);
         }
       }
 
     // Push any varargs arguments on the list. Don't forget their attributes.
     for (CallSite::arg_iterator E = CS.arg_end(); I != E; ++I, ++i) {
       Args.push_back(*I);
-      if (CallPAL.hasAttributes(i + 1)) {
+      AttributesVec.push_back(CallPAL.getParamAttributes(i + 1));
-        AttrBuilder B(CallPAL, i + 1);
-        AttributesVec.push_back(
-            AttributeList::get(F->getContext(), Args.size(), B));
-      }
     }
 
-    if (CallPAL.hasAttributes(AttributeList::FunctionIndex))
+    AttributesVec.push_back(CallPAL.getFnAttributes());
-      AttributesVec.push_back(
-          AttributeList::get(Call->getContext(), CallPAL.getFnAttributes()));
 
     // Reconstruct the AttributesList based on the vector we constructed.
     AttributeList NewCallPAL =

lib/Transforms/IPO/MergeFunctions.cpp

@@ -439,8 +439,7 @@ void MergeFunctions::replaceDirectCallers(Function *Old, Function *New) {
           Context, AttributeList::ReturnIndex, NewFuncAttrs.getRetAttributes());
 
       for (unsigned argIdx = 0; argIdx < CS.arg_size(); argIdx++) {
-        AttributeList Attrs = NewFuncAttrs.getParamAttributes(argIdx);
+        if (AttributeSetNode *Attrs = NewFuncAttrs.getParamAttributes(argIdx))
-        if (Attrs.getNumSlots())
           CallSiteAttrs = CallSiteAttrs.addAttributes(Context, argIdx, Attrs);
       }
 

lib/Transforms/InstCombine/InstCombineCalls.cpp

@@ -23,6 +23,7 @@
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/Analysis/ValueTracking.h"
+#include "llvm/IR/AttributeSetNode.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constant.h"
@@ -3992,7 +3993,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
     if (!CastInst::isBitOrNoopPointerCastable(ActTy, ParamTy, DL))
       return false;   // Cannot transform this parameter value.
 
-    if (AttrBuilder(CallerPAL.getParamAttributes(i + 1), i + 1).
+    if (AttrBuilder(CallerPAL.getParamAttributes(i + 1)).
           overlaps(AttributeFuncs::typeIncompatible(ParamTy)))
       return false;   // Attribute not compatible with transformed value.
 
@@ -4001,9 +4002,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
 
     // If the parameter is passed as a byval argument, then we have to have a
     // sized type and the sized type has to have the same size as the old type.
-    if (ParamTy != ActTy &&
+    if (ParamTy != ActTy && CallerPAL.hasAttribute(i + 1, Attribute::ByVal)) {
-        CallerPAL.getParamAttributes(i + 1).hasAttribute(i + 1,
-                                                         Attribute::ByVal)) {
       PointerType *ParamPTy = dyn_cast<PointerType>(ParamTy);
       if (!ParamPTy || !ParamPTy->getElementType()->isSized())
         return false;
@@ -4084,7 +4083,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
     }
 
     // Add any parameter attributes.
-    AttrBuilder PAttrs(CallerPAL.getParamAttributes(i + 1), i + 1);
+    AttrBuilder PAttrs(CallerPAL.getParamAttributes(i + 1));
     if (PAttrs.hasAttributes())
       attrVec.push_back(
           AttributeList::get(Caller->getContext(), i + 1, PAttrs));
@@ -4112,7 +4111,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
         }
 
         // Add any parameter attributes.
-        AttrBuilder PAttrs(CallerPAL.getParamAttributes(i + 1), i + 1);
+        AttrBuilder PAttrs(CallerPAL.getParamAttributes(i + 1));
         if (PAttrs.hasAttributes())
           attrVec.push_back(
               AttributeList::get(FT->getContext(), i + 1, PAttrs));
@@ -4120,9 +4119,11 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
     }
   }
 
-  AttributeList FnAttrs = CallerPAL.getFnAttributes();
+  AttributeSetNode *FnAttrs = CallerPAL.getFnAttributes();
   if (CallerPAL.hasAttributes(AttributeList::FunctionIndex))
-    attrVec.push_back(AttributeList::get(Callee->getContext(), FnAttrs));
+    attrVec.push_back(AttributeList::get(Callee->getContext(),
+                                         AttributeList::FunctionIndex,
+                                         AttrBuilder(FnAttrs)));
 
   if (NewRetTy->isVoidTy())
     Caller->setName("");   // Void type should not have a name.
@@ -4200,7 +4201,7 @@ InstCombiner::transformCallThroughTrampoline(CallSite CS,
   Value *Callee = CS.getCalledValue();
   PointerType *PTy = cast<PointerType>(Callee->getType());
   FunctionType *FTy = cast<FunctionType>(PTy->getElementType());
-  const AttributeList &Attrs = CS.getAttributes();
+  AttributeList Attrs = CS.getAttributes();
 
   // If the call already has the 'nest' attribute somewhere then give up -
   // otherwise 'nest' would occur twice after splicing in the chain.
@@ -4213,11 +4214,11 @@ InstCombiner::transformCallThroughTrampoline(CallSite CS,
   Function *NestF =cast<Function>(Tramp->getArgOperand(1)->stripPointerCasts());
   FunctionType *NestFTy = cast<FunctionType>(NestF->getValueType());
 
-  const AttributeList &NestAttrs = NestF->getAttributes();
+  AttributeList NestAttrs = NestF->getAttributes();
   if (!NestAttrs.isEmpty()) {
     unsigned NestIdx = 1;
     Type *NestTy = nullptr;
-    AttributeList NestAttr;
+    AttributeSetNode *NestAttr;
 
     // Look for a parameter marked with the 'nest' attribute.
     for (FunctionType::param_iterator I = NestFTy->param_begin(),
@@ -4232,18 +4233,15 @@ InstCombiner::transformCallThroughTrampoline(CallSite CS,
     if (NestTy) {
       Instruction *Caller = CS.getInstruction();
       std::vector<Value*> NewArgs;
+      std::vector<AttributeSetNode *> NewAttrs;
       NewArgs.reserve(CS.arg_size() + 1);
-
+      NewAttrs.reserve(CS.arg_size() + 2);
-      SmallVector<AttributeList, 8> NewAttrs;
-      NewAttrs.reserve(Attrs.getNumSlots() + 1);
 
       // Insert the nest argument into the call argument list, which may
       // mean appending it.  Likewise for attributes.
 
       // Add any result attributes.
-      if (Attrs.hasAttributes(AttributeList::ReturnIndex))
+      NewAttrs.push_back(Attrs.getRetAttributes());
-        NewAttrs.push_back(
-            AttributeList::get(Caller->getContext(), Attrs.getRetAttributes()));
 
       {
         unsigned Idx = 1;
@@ -4255,8 +4253,7 @@ InstCombiner::transformCallThroughTrampoline(CallSite CS,
             if (NestVal->getType() != NestTy)
               NestVal = Builder->CreateBitCast(NestVal, NestTy, "nest");
             NewArgs.push_back(NestVal);
-            NewAttrs.push_back(
+            NewAttrs.push_back(NestAttr);
-                AttributeList::get(Caller->getContext(), NestAttr));
           }
 
           if (I == E)
@@ -4264,12 +4261,7 @@ InstCombiner::transformCallThroughTrampoline(CallSite CS,
 
           // Add the original argument and attributes.
           NewArgs.push_back(*I);
-          AttributeList Attr = Attrs.getParamAttributes(Idx);
+          NewAttrs.push_back(Attrs.getParamAttributes(Idx));
-          if (Attr.hasAttributes(Idx)) {
-            AttrBuilder B(Attr, Idx);
-            NewAttrs.push_back(AttributeList::get(Caller->getContext(),
-                                                  Idx + (Idx >= NestIdx), B));
-          }
 
           ++Idx;
           ++I;
@@ -4277,9 +4269,7 @@ InstCombiner::transformCallThroughTrampoline(CallSite CS,
       }
 
       // Add any function attributes.
-      if (Attrs.hasAttributes(AttributeList::FunctionIndex))
+      NewAttrs.push_back(Attrs.getFnAttributes());
-        NewAttrs.push_back(
-            AttributeList::get(FTy->getContext(), Attrs.getFnAttributes()));
 
       // The trampoline may have been bitcast to a bogus type (FTy).
       // Handle this by synthesizing a new function type, equal to FTy
@@ -4319,8 +4309,7 @@ InstCombiner::transformCallThroughTrampoline(CallSite CS,
         NestF->getType() == PointerType::getUnqual(NewFTy) ?
         NestF : ConstantExpr::getBitCast(NestF,
                                          PointerType::getUnqual(NewFTy));
-      const AttributeList &NewPAL =
+      AttributeList NewPAL = AttributeList::get(FTy->getContext(), NewAttrs);
-          AttributeList::get(FTy->getContext(), NewAttrs);
 
       SmallVector<OperandBundleDef, 1> OpBundles;
       CS.getOperandBundlesAsDefs(OpBundles);

lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

@@ -1392,7 +1392,6 @@ makeStatepointExplicitImpl(const CallSite CS, /* to replace */
 
   // Create the statepoint given all the arguments
   Instruction *Token = nullptr;
-  AttributeList ReturnAttrs;
   if (CS.isCall()) {
     CallInst *ToReplace = cast<CallInst>(CS.getInstruction());
     CallInst *Call = Builder.CreateGCStatepointCall(
@@ -1407,8 +1406,9 @@ makeStatepointExplicitImpl(const CallSite CS, /* to replace */
     AttributeList NewAttrs = legalizeCallAttributes(ToReplace->getAttributes());
     // In case if we can handle this set of attributes - set up function attrs
     // directly on statepoint and return attrs later for gc_result intrinsic.
-    Call->setAttributes(NewAttrs.getFnAttributes());
+    Call->setAttributes(AttributeList::get(Call->getContext(),
-    ReturnAttrs = NewAttrs.getRetAttributes();
+                                           AttributeList::FunctionIndex,
+                                           NewAttrs.getFnAttributes()));
 
     Token = Call;
 
@@ -1435,8 +1435,9 @@ makeStatepointExplicitImpl(const CallSite CS, /* to replace */
     AttributeList NewAttrs = legalizeCallAttributes(ToReplace->getAttributes());
     // In case if we can handle this set of attributes - set up function attrs
     // directly on statepoint and return attrs later for gc_result intrinsic.
-    Invoke->setAttributes(NewAttrs.getFnAttributes());
+    Invoke->setAttributes(AttributeList::get(Invoke->getContext(),
-    ReturnAttrs = NewAttrs.getRetAttributes();
+                                             AttributeList::FunctionIndex,
+                                             NewAttrs.getFnAttributes()));
 
     Token = Invoke;
 
@@ -1482,7 +1483,9 @@ makeStatepointExplicitImpl(const CallSite CS, /* to replace */
       StringRef Name =
           CS.getInstruction()->hasName() ? CS.getInstruction()->getName() : "";
       CallInst *GCResult = Builder.CreateGCResult(Token, CS.getType(), Name);
-      GCResult->setAttributes(CS.getAttributes().getRetAttributes());
+      GCResult->setAttributes(
+          AttributeList::get(GCResult->getContext(), AttributeList::ReturnIndex,
+                             CS.getAttributes().getRetAttributes()));
 
       // We cannot RAUW or delete CS.getInstruction() because it could be in the
       // live set of some other safepoint, in which case that safepoint's

lib/Transforms/Utils/CloneFunction.cpp

@@ -103,21 +103,25 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
                  ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
                  TypeMapper, Materializer));
 
+  SmallVector<std::pair<unsigned, AttributeSetNode*>, 4> AttrVec;
   AttributeList OldAttrs = OldFunc->getAttributes();
+
+  // Copy the return attributes.
+  if (auto *RetAttrs = OldAttrs.getRetAttributes())
+    AttrVec.emplace_back(AttributeList::ReturnIndex, RetAttrs);
+
   // Clone any argument attributes that are present in the VMap.
   for (const Argument &OldArg : OldFunc->args())
     if (Argument *NewArg = dyn_cast<Argument>(VMap[&OldArg])) {
-      AttributeList attrs = OldAttrs.getParamAttributes(OldArg.getArgNo() + 1);
+      if (auto *ParmAttrs = OldAttrs.getParamAttributes(OldArg.getArgNo() + 1))
-      if (attrs.getNumSlots() > 0)
+        AttrVec.emplace_back(NewArg->getArgNo() + 1, ParmAttrs);
-        NewArg->addAttr(attrs);
     }
 
-  NewFunc->setAttributes(
+  // Copy any function attributes.
-      NewFunc->getAttributes()
+  if (auto *FnAttrs = OldAttrs.getFnAttributes())
-          .addAttributes(NewFunc->getContext(), AttributeList::ReturnIndex,
+    AttrVec.emplace_back(AttributeList::FunctionIndex, FnAttrs);
-                         OldAttrs.getRetAttributes())
+
-          .addAttributes(NewFunc->getContext(), AttributeList::FunctionIndex,
+  NewFunc->setAttributes(AttributeList::get(NewFunc->getContext(), AttrVec));
-                         OldAttrs.getFnAttributes()));
 
   SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
   OldFunc->getAllMetadata(MDs);

lib/Transforms/Utils/CodeExtractor.cpp

@@ -362,8 +362,7 @@ Function *CodeExtractor::constructFunction(const ValueSet &inputs,
   //  "target-features" attribute allowing it to be lowered.
   // FIXME: This should be changed to check to see if a specific
   //           attribute can not be inherited.
-  AttributeList OldFnAttrs = oldFunction->getAttributes().getFnAttributes();
+  AttrBuilder AB(oldFunction->getAttributes().getFnAttributes());
-  AttrBuilder AB(OldFnAttrs, AttributeList::FunctionIndex);
   for (const auto &Attr : AB.td_attrs())
     newFunction->addFnAttr(Attr.first, Attr.second);