commit eb9dd5b87f43cbd9641fba0555b775589e49af33
author Reid Kleckner <rnk@google.com> Mon Apr 10 23:31:05 2017 +0000
committer Reid Kleckner <rnk@google.com> Mon Apr 10 23:31:05 2017 +0000
tree 3aed507a5bb8b601a92c13399bf2a54d54fed432
parent 75696ffa2533abb4e4db406d4e2de42e86197413

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

llvm/lib/Transforms/Utils/CloneFunction.cpp

diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 60b988e..f480302 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -103,21 +103,25 @@
                  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 (attrs.getNumSlots() > 0)
-        NewArg->addAttr(attrs);
+      if (auto *ParmAttrs = OldAttrs.getParamAttributes(OldArg.getArgNo() + 1))
+        AttrVec.emplace_back(NewArg->getArgNo() + 1, ParmAttrs);
     }
 
-  NewFunc->setAttributes(
-      NewFunc->getAttributes()
-          .addAttributes(NewFunc->getContext(), AttributeList::ReturnIndex,
-                         OldAttrs.getRetAttributes())
-          .addAttributes(NewFunc->getContext(), AttributeList::FunctionIndex,
-                         OldAttrs.getFnAttributes()));
+  // Copy any function attributes.
+  if (auto *FnAttrs = OldAttrs.getFnAttributes())
+    AttrVec.emplace_back(AttributeList::FunctionIndex, FnAttrs);
+
+  NewFunc->setAttributes(AttributeList::get(NewFunc->getContext(), AttrVec));
 
   SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
   OldFunc->getAllMetadata(MDs);

llvm/lib/Transforms/Utils/CodeExtractor.cpp

diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
index a7f7993..644d93b 100644
--- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -362,8 +362,7 @@
   //  "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(OldFnAttrs, AttributeList::FunctionIndex);
+  AttrBuilder AB(oldFunction->getAttributes().getFnAttributes());
   for (const auto &Attr : AB.td_attrs())
     newFunction->addFnAttr(Attr.first, Attr.second);