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
diff --git a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
index fe79efc..c7138bf 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 @@
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 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 @@
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 @@
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 @@
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 @@
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 =