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

3 files changed, 50 insertions, 81 deletions

diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index b7fe2d6f7a2..1646237b096 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/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(
-        AttributeList::get(F->getContext(), PAL.getRetAttributes()));
+  AttributesVec.push_back(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);
-      if (attrs.hasAttributes(ArgIndex)) {
-        AttrBuilder B(attrs, ArgIndex);
-        AttributesVec.push_back(
-            AttributeList::get(F->getContext(), Params.size(), B));
-      }
+      AttributesVec.push_back(PAL.getParamAttributes(ArgIndex));
     } 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(
-        AttributeList::get(FTy->getContext(), PAL.getFnAttributes()));
+  AttributesVec.push_back(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(
-          AttributeList::get(F->getContext(), CallPAL.getRetAttributes()));
+    AttributesVec.push_back(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
-
-        if (CallPAL.hasAttributes(ArgIndex)) {
-          AttrBuilder B(CallPAL, ArgIndex);
-          AttributesVec.push_back(
-              AttributeList::get(F->getContext(), Args.size(), B));
-        }
+        AttributesVec.push_back(CallPAL.getAttributes(ArgIndex));
       } 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)) {
-        AttrBuilder B(CallPAL, ArgIndex);
-        AttributesVec.push_back(
-            AttributeList::get(F->getContext(), Args.size(), B));
-      }
+      AttributesVec.push_back(CallPAL.getAttributes(ArgIndex));
     }
 
     // Add any function attributes.
-    if (CallPAL.hasAttributes(AttributeList::FunctionIndex))
-      AttributesVec.push_back(
-          AttributeList::get(Call->getContext(), CallPAL.getFnAttributes()));
+    AttributesVec.push_back(CallPAL.getFnAttributes());
 
     SmallVector<OperandBundleDef, 1> OpBundles;
     CS.getOperandBundlesAsDefs(OpBundles);
diff --git a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
index fe79efce290..c7138bfbc8b 100644
--- a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
+++ b/llvm/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(
-            AttributeList::get(Fn.getContext(), PAL.getFnAttributes()));
+        AttributesVec.push_back(AttributeList::get(Fn.getContext(),
+                                                   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;
-
-      // Get the original parameter attributes (skipping the first one, that is
-      // 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));
-      }
+      AttributesVec.push_back(PAL.getParamAttributes(i + 1));
+      HasLiveReturnedArg |= PAL.hasAttribute(i + 1, Attribute::Returned);
     } 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(),
-                                     AttributeList::ReturnIndex,
-                                     AttributeFuncs::typeIncompatible(NRetTy));
+    RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy));
   else
-    assert(!AttrBuilder(RAttrs, AttributeList::ReturnIndex)
-                .overlaps(AttributeFuncs::typeIncompatible(NRetTy)) &&
+    assert(!RAttrs.overlaps(AttributeFuncs::typeIncompatible(NRetTy)) &&
            "Return attributes no longer compatible?");
 
-  if (RAttrs.hasAttributes(AttributeList::ReturnIndex))
-    AttributesVec.push_back(AttributeList::get(NRetTy->getContext(), RAttrs));
+  AttributesVec[0] = AttributeSetNode::get(F->getContext(), RAttrs);
 
-  if (PAL.hasAttributes(AttributeList::FunctionIndex))
-    AttributesVec.push_back(
-        AttributeList::get(F->getContext(), PAL.getFnAttributes()));
+  // Transfer the function attributes, if any.
+  AttributesVec.push_back(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.
-    AttributeList RAttrs = CallPAL.getRetAttributes();
-
-    // Adjust in case the function was changed to return void.
-    RAttrs = RAttrs.removeAttributes(
-        NRetTy->getContext(), AttributeList::ReturnIndex,
-        AttributeFuncs::typeIncompatible(NF->getReturnType()));
-    if (RAttrs.hasAttributes(AttributeList::ReturnIndex))
-      AttributesVec.push_back(AttributeList::get(NF->getContext(), RAttrs));
+    // Adjust the call return attributes in case the function was changed to
+    // return void.
+    AttrBuilder RAttrs(CallPAL.getRetAttributes());
+    RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy));
+    AttributesVec.push_back(AttributeSetNode::get(F->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)) {
-          AttrBuilder B(CallPAL, i + 1);
+        AttributeSetNode *Attrs = CallPAL.getParamAttributes(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
-          // and this is not an expected case anyway
-          if (NRetTy != RetTy && B.contains(Attribute::Returned))
-            B.removeAttribute(Attribute::Returned);
-          AttributesVec.push_back(
-              AttributeList::get(F->getContext(), Args.size(), B));
+          // attributes on function declaration but it's less clearly a win and
+          // this is not an expected case anyway
+          AttributesVec.push_back(AttributeSetNode::get(
+              F->getContext(),
+              AttrBuilder(Attrs).removeAttribute(Attribute::Returned)));
+        } 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)) {
-        AttrBuilder B(CallPAL, i + 1);
-        AttributesVec.push_back(
-            AttributeList::get(F->getContext(), Args.size(), B));
-      }
+      AttributesVec.push_back(CallPAL.getParamAttributes(i + 1));
     }
 
-    if (CallPAL.hasAttributes(AttributeList::FunctionIndex))
-      AttributesVec.push_back(
-          AttributeList::get(Call->getContext(), CallPAL.getFnAttributes()));
+    AttributesVec.push_back(CallPAL.getFnAttributes());
 
     // Reconstruct the AttributesList based on the vector we constructed.
     AttributeList NewCallPAL =
diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index dc7390232c5..5d41ca9f9fc 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/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 (Attrs.getNumSlots())
+        if (AttributeSetNode *Attrs = NewFuncAttrs.getParamAttributes(argIdx))
           CallSiteAttrs = CallSiteAttrs.addAttributes(Context, argIdx, Attrs);
       }