From dfc1ffb1c979d8b7651a1aa98cf2d354cd8b6af4 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Fri, 28 Apr 2017 23:01:32 +0000 Subject: [PATCH] Revert r301697 "[IR] Make add/remove Attributes use AttrBuilder instead of AttributeList" This broke the Clang build. (Clang-side patch missing?) Original commit message: > [IR] Make add/remove Attributes use AttrBuilder instead of > AttributeList > > This change cleans up call sites and avoids creating temporary > AttributeList objects. > > NFC git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@301712 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/CommandFlags.h | 30 ++++++++----- include/llvm/IR/Attributes.h | 11 ++++- include/llvm/IR/Function.h | 4 +- lib/CodeGen/GlobalISel/CallLowering.cpp | 4 +- lib/IR/Attributes.cpp | 43 +++++++++++-------- lib/IR/Function.cpp | 4 +- lib/Target/Mips/Mips16HardFloat.cpp | 9 ++-- lib/Transforms/Coroutines/CoroSplit.cpp | 4 +- .../Instrumentation/DataFlowSanitizer.cpp | 21 ++++++--- .../Instrumentation/MemorySanitizer.cpp | 9 +++- .../Scalar/RewriteStatepointsForGC.cpp | 3 +- unittests/IR/AttributesTest.cpp | 2 +- 12 files changed, 93 insertions(+), 51 deletions(-) diff --git a/include/llvm/CodeGen/CommandFlags.h b/include/llvm/CodeGen/CommandFlags.h index 0d898827efc6..317a5d3f54c8 100644 --- a/include/llvm/CodeGen/CommandFlags.h +++ b/include/llvm/CodeGen/CommandFlags.h @@ -346,21 +346,29 @@ static inline void setFunctionAttributes(StringRef CPU, StringRef Features, Module &M) { for (auto &F : M) { auto &Ctx = F.getContext(); - AttributeList Attrs = F.getAttributes(); - AttrBuilder NewAttrs; + AttributeList Attrs = F.getAttributes(), NewAttrs; if (!CPU.empty()) - NewAttrs.addAttribute("target-cpu", CPU); + NewAttrs = NewAttrs.addAttribute(Ctx, AttributeList::FunctionIndex, + "target-cpu", CPU); + if (!Features.empty()) - NewAttrs.addAttribute("target-features", Features); + NewAttrs = NewAttrs.addAttribute(Ctx, AttributeList::FunctionIndex, + "target-features", Features); + if (DisableFPElim.getNumOccurrences() > 0) - NewAttrs.addAttribute("no-frame-pointer-elim", - DisableFPElim ? "true" : "false"); + NewAttrs = NewAttrs.addAttribute(Ctx, AttributeList::FunctionIndex, + "no-frame-pointer-elim", + DisableFPElim ? "true" : "false"); + if (DisableTailCalls.getNumOccurrences() > 0) - NewAttrs.addAttribute("disable-tail-calls", - toStringRef(DisableTailCalls)); + NewAttrs = NewAttrs.addAttribute(Ctx, AttributeList::FunctionIndex, + "disable-tail-calls", + toStringRef(DisableTailCalls)); + if (StackRealign) - NewAttrs.addAttribute("stackrealign"); + NewAttrs = NewAttrs.addAttribute(Ctx, AttributeList::FunctionIndex, + "stackrealign"); if (TrapFuncName.getNumOccurrences() > 0) for (auto &B : F) @@ -374,8 +382,8 @@ static inline void setFunctionAttributes(StringRef CPU, StringRef Features, Attribute::get(Ctx, "trap-func-name", TrapFuncName)); // Let NewAttrs override Attrs. - F.setAttributes( - Attrs.addAttributes(Ctx, AttributeList::FunctionIndex, NewAttrs)); + NewAttrs = Attrs.addAttributes(Ctx, AttributeList::FunctionIndex, NewAttrs); + F.setAttributes(NewAttrs); } } diff --git a/include/llvm/IR/Attributes.h b/include/llvm/IR/Attributes.h index c49e19af78e8..af46034d5a9e 100644 --- a/include/llvm/IR/Attributes.h +++ b/include/llvm/IR/Attributes.h @@ -353,6 +353,9 @@ class AttributeList { /// \brief Add attributes to the attribute set at the given index. Because /// attribute sets are immutable, this returns a new set. + AttributeList addAttributes(LLVMContext &C, unsigned Index, + AttributeList Attrs) const; + AttributeList addAttributes(LLVMContext &C, unsigned Index, const AttrBuilder &B) const; @@ -372,7 +375,13 @@ class AttributeList { /// attribute list. Because attribute lists are immutable, this returns the /// new list. AttributeList removeAttributes(LLVMContext &C, unsigned Index, - const AttrBuilder &AttrsToRemove) const; + AttributeList Attrs) const; + + /// \brief Remove the specified 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 AttrBuilder &Attrs) const; /// \brief Remove all attributes at the specified index from this /// attribute list. Because attribute lists are immutable, this returns the diff --git a/include/llvm/IR/Function.h b/include/llvm/IR/Function.h index 8cda785ef209..9e723f977755 100644 --- a/include/llvm/IR/Function.h +++ b/include/llvm/IR/Function.h @@ -279,7 +279,7 @@ class Function : public GlobalObject, public ilist_node { void addAttribute(unsigned i, Attribute Attr); /// @brief adds the attributes to the list of attributes. - void addAttributes(unsigned i, const AttrBuilder &Attrs); + void addAttributes(unsigned i, AttributeList Attrs); /// @brief removes the attribute from the list of attributes. void removeAttribute(unsigned i, Attribute::AttrKind Kind); @@ -288,7 +288,7 @@ class Function : public GlobalObject, public ilist_node { void removeAttribute(unsigned i, StringRef Kind); /// @brief removes the attributes from the list of attributes. - void removeAttributes(unsigned i, const AttrBuilder &Attrs); + void removeAttributes(unsigned i, AttributeList Attrs); /// @brief check if an attributes is in the list of attributes. bool hasAttribute(unsigned i, Attribute::AttrKind Kind) const { diff --git a/lib/CodeGen/GlobalISel/CallLowering.cpp b/lib/CodeGen/GlobalISel/CallLowering.cpp index 47433eb126d4..ebfe6cb3b733 100644 --- a/lib/CodeGen/GlobalISel/CallLowering.cpp +++ b/lib/CodeGen/GlobalISel/CallLowering.cpp @@ -83,8 +83,8 @@ void CallLowering::setArgFlags(CallLowering::ArgInfo &Arg, unsigned OpIdx, // For ByVal, alignment should be passed from FE. BE will guess if // this info is not there but there are cases it cannot get right. unsigned FrameAlign; - if (FuncInfo.getParamAlignment(OpIdx - 2)) - FrameAlign = FuncInfo.getParamAlignment(OpIdx - 2); + if (FuncInfo.getParamAlignment(OpIdx - 1)) + FrameAlign = FuncInfo.getParamAlignment(OpIdx - 1); else FrameAlign = getTLI()->getByValTypeAlignment(ElementTy, DL); Arg.Flags.setByValAlign(FrameAlign); diff --git a/lib/IR/Attributes.cpp b/lib/IR/Attributes.cpp index 3d08ecdd484e..62f127bd02e0 100644 --- a/lib/IR/Attributes.cpp +++ b/lib/IR/Attributes.cpp @@ -936,9 +936,7 @@ AttributeList AttributeList::get(LLVMContext &C, AttributeList AttributeList::addAttribute(LLVMContext &C, unsigned Index, Attribute::AttrKind Kind) const { if (hasAttribute(Index, Kind)) return *this; - AttrBuilder B; - B.addAttribute(Kind); - return addAttributes(C, Index, B); + return addAttributes(C, Index, AttributeList::get(C, Index, Kind)); } AttributeList AttributeList::addAttribute(LLVMContext &C, unsigned Index, @@ -946,7 +944,7 @@ AttributeList AttributeList::addAttribute(LLVMContext &C, unsigned Index, StringRef Value) const { AttrBuilder B; B.addAttribute(Kind, Value); - return addAttributes(C, Index, B); + return addAttributes(C, Index, AttributeList::get(C, Index, B)); } AttributeList AttributeList::addAttribute(LLVMContext &C, @@ -979,6 +977,14 @@ AttributeList AttributeList::addAttribute(LLVMContext &C, return get(C, AttrVec); } +AttributeList AttributeList::addAttributes(LLVMContext &C, unsigned Index, + AttributeList Attrs) const { + if (!pImpl) return Attrs; + if (!Attrs.pImpl) return *this; + + return addAttributes(C, Index, Attrs.getAttributes(Index)); +} + AttributeList AttributeList::addAttributes(LLVMContext &C, unsigned Index, const AttrBuilder &B) const { if (!B.hasAttributes()) @@ -1028,17 +1034,18 @@ AttributeList AttributeList::addAttributes(LLVMContext &C, unsigned Index, AttributeList AttributeList::removeAttribute(LLVMContext &C, unsigned Index, Attribute::AttrKind Kind) const { if (!hasAttribute(Index, Kind)) return *this; - AttrBuilder B; - B.addAttribute(Kind); - return removeAttributes(C, Index, B); + return removeAttributes(C, Index, AttributeList::get(C, Index, Kind)); } AttributeList AttributeList::removeAttribute(LLVMContext &C, unsigned Index, StringRef Kind) const { if (!hasAttribute(Index, Kind)) return *this; - AttrBuilder B; - B.addAttribute(Kind); - return removeAttributes(C, Index, B); + return removeAttributes(C, Index, AttributeList::get(C, Index, Kind)); +} + +AttributeList AttributeList::removeAttributes(LLVMContext &C, unsigned Index, + AttributeList Attrs) const { + return removeAttributes(C, Index, AttrBuilder(Attrs.getAttributes(Index))); } AttributeList AttributeList::removeAttributes(LLVMContext &C, unsigned Index, @@ -1096,7 +1103,7 @@ AttributeList AttributeList::addDereferenceableAttr(LLVMContext &C, uint64_t Bytes) const { AttrBuilder B; B.addDereferenceableAttr(Bytes); - return addAttributes(C, Index, B); + return addAttributes(C, Index, AttributeList::get(C, Index, B)); } AttributeList @@ -1104,7 +1111,7 @@ AttributeList::addDereferenceableOrNullAttr(LLVMContext &C, unsigned Index, uint64_t Bytes) const { AttrBuilder B; B.addDereferenceableOrNullAttr(Bytes); - return addAttributes(C, Index, B); + return addAttributes(C, Index, AttributeList::get(C, Index, B)); } AttributeList @@ -1113,7 +1120,7 @@ AttributeList::addAllocSizeAttr(LLVMContext &C, unsigned Index, const Optional &NumElemsArg) { AttrBuilder B; B.addAllocSizeAttr(ElemSizeArg, NumElemsArg); - return addAttributes(C, Index, B); + return addAttributes(C, Index, AttributeList::get(C, Index, B)); } //===----------------------------------------------------------------------===// @@ -1603,10 +1610,12 @@ static void adjustCallerSSPLevel(Function &Caller, const Function &Callee) { // If upgrading the SSP attribute, clear out the old SSP Attributes first. // Having multiple SSP attributes doesn't actually hurt, but it adds useless // clutter to the IR. - AttrBuilder OldSSPAttr; - OldSSPAttr.addAttribute(Attribute::StackProtect) - .addAttribute(Attribute::StackProtectStrong) - .addAttribute(Attribute::StackProtectReq); + AttrBuilder B; + B.addAttribute(Attribute::StackProtect) + .addAttribute(Attribute::StackProtectStrong) + .addAttribute(Attribute::StackProtectReq); + AttributeList OldSSPAttr = + AttributeList::get(Caller.getContext(), AttributeList::FunctionIndex, B); if (Callee.hasFnAttribute(Attribute::StackProtectReq)) { Caller.removeAttributes(AttributeList::FunctionIndex, OldSSPAttr); diff --git a/lib/IR/Function.cpp b/lib/IR/Function.cpp index 7b0838c41ab5..fc61ba7439ba 100644 --- a/lib/IR/Function.cpp +++ b/lib/IR/Function.cpp @@ -328,7 +328,7 @@ void Function::addAttribute(unsigned i, Attribute Attr) { setAttributes(PAL); } -void Function::addAttributes(unsigned i, const AttrBuilder &Attrs) { +void Function::addAttributes(unsigned i, AttributeList Attrs) { AttributeList PAL = getAttributes(); PAL = PAL.addAttributes(getContext(), i, Attrs); setAttributes(PAL); @@ -346,7 +346,7 @@ void Function::removeAttribute(unsigned i, StringRef Kind) { setAttributes(PAL); } -void Function::removeAttributes(unsigned i, const AttrBuilder &Attrs) { +void Function::removeAttributes(unsigned i, AttributeList Attrs) { AttributeList PAL = getAttributes(); PAL = PAL.removeAttributes(getContext(), i, Attrs); setAttributes(PAL); diff --git a/lib/Target/Mips/Mips16HardFloat.cpp b/lib/Target/Mips/Mips16HardFloat.cpp index 5a394fe02f16..a71b161b24cc 100644 --- a/lib/Target/Mips/Mips16HardFloat.cpp +++ b/lib/Target/Mips/Mips16HardFloat.cpp @@ -490,14 +490,15 @@ static void createFPFnStub(Function *F, Module *M, FPParamVariant PV, // remove the use-soft-float attribute // static void removeUseSoftFloat(Function &F) { - AttrBuilder B; + AttributeList A; DEBUG(errs() << "removing -use-soft-float\n"); - B.addAttribute("use-soft-float", "false"); - F.removeAttributes(AttributeList::FunctionIndex, B); + A = A.addAttribute(F.getContext(), AttributeList::FunctionIndex, + "use-soft-float", "false"); + F.removeAttributes(AttributeList::FunctionIndex, A); if (F.hasFnAttribute("use-soft-float")) { DEBUG(errs() << "still has -use-soft-float\n"); } - F.addAttributes(AttributeList::FunctionIndex, B); + F.addAttributes(AttributeList::FunctionIndex, A); } diff --git a/lib/Transforms/Coroutines/CoroSplit.cpp b/lib/Transforms/Coroutines/CoroSplit.cpp index f5bb5cc274d1..ab648f884c5b 100644 --- a/lib/Transforms/Coroutines/CoroSplit.cpp +++ b/lib/Transforms/Coroutines/CoroSplit.cpp @@ -245,7 +245,9 @@ static Function *createClone(Function &F, Twine Suffix, coro::Shape &Shape, // Remove old return attributes. NewF->removeAttributes( AttributeList::ReturnIndex, - AttributeFuncs::typeIncompatible(NewF->getReturnType())); + AttributeList::get( + NewF->getContext(), AttributeList::ReturnIndex, + AttributeFuncs::typeIncompatible(NewF->getReturnType()))); // Make AllocaSpillBlock the new entry block. auto *SwitchBB = cast(VMap[ResumeEntry]); diff --git a/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp b/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp index 9fe6f87d4fbe..4e454f0c95b6 100644 --- a/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp +++ b/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp @@ -254,7 +254,7 @@ class DataFlowSanitizer : public ModulePass { MDNode *ColdCallWeights; DFSanABIList ABIList; DenseMap UnwrappedFnMap; - AttrBuilder ReadOnlyNoneAttrs; + AttributeList ReadOnlyNoneAttrs; bool DFSanRuntimeShadowMask; Value *getShadowAddress(Value *Addr, Instruction *Pos); @@ -544,12 +544,16 @@ DataFlowSanitizer::buildWrapperFunction(Function *F, StringRef NewFName, NewF->copyAttributesFrom(F); NewF->removeAttributes( AttributeList::ReturnIndex, - AttributeFuncs::typeIncompatible(NewFT->getReturnType())); + AttributeList::get( + F->getContext(), AttributeList::ReturnIndex, + AttributeFuncs::typeIncompatible(NewFT->getReturnType()))); BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", NewF); if (F->isVarArg()) { - NewF->removeAttributes(AttributeList::FunctionIndex, - AttrBuilder().addAttribute("split-stack")); + NewF->removeAttributes( + AttributeList::FunctionIndex, + AttributeList().addAttribute(*Ctx, AttributeList::FunctionIndex, + "split-stack")); CallInst::Create(DFSanVarargWrapperFn, IRBuilder<>(BB).CreateGlobalStringPtr(F->getName()), "", BB); @@ -694,8 +698,9 @@ bool DataFlowSanitizer::runOnModule(Module &M) { } } - ReadOnlyNoneAttrs.addAttribute(Attribute::ReadOnly) - .addAttribute(Attribute::ReadNone); + AttrBuilder B; + B.addAttribute(Attribute::ReadOnly).addAttribute(Attribute::ReadNone); + ReadOnlyNoneAttrs = AttributeList::get(*Ctx, AttributeList::FunctionIndex, B); // First, change the ABI of every function in the module. ABI-listed // functions keep their original ABI and get a wrapper function. @@ -717,7 +722,9 @@ bool DataFlowSanitizer::runOnModule(Module &M) { NewF->copyAttributesFrom(&F); NewF->removeAttributes( AttributeList::ReturnIndex, - AttributeFuncs::typeIncompatible(NewFT->getReturnType())); + AttributeList::get( + NewF->getContext(), AttributeList::ReturnIndex, + AttributeFuncs::typeIncompatible(NewFT->getReturnType()))); for (Function::arg_iterator FArg = F.arg_begin(), NewFArg = NewF->arg_begin(), FArgEnd = F.arg_end(); diff --git a/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 15333a5317dd..3e480a6df446 100644 --- a/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -2607,7 +2607,10 @@ struct MemorySanitizerVisitor : public InstVisitor { AttrBuilder B; B.addAttribute(Attribute::ReadOnly) .addAttribute(Attribute::ReadNone); - Func->removeAttributes(AttributeList::FunctionIndex, B); + Func->removeAttributes(AttributeList::FunctionIndex, + AttributeList::get(Func->getContext(), + AttributeList::FunctionIndex, + B)); } maybeMarkSanitizerLibraryCallNoBuiltin(Call, TLI); @@ -3656,7 +3659,9 @@ bool MemorySanitizer::runOnFunction(Function &F) { AttrBuilder B; B.addAttribute(Attribute::ReadOnly) .addAttribute(Attribute::ReadNone); - F.removeAttributes(AttributeList::FunctionIndex, B); + F.removeAttributes( + AttributeList::FunctionIndex, + AttributeList::get(F.getContext(), AttributeList::FunctionIndex, B)); return Visitor.runOnFunction(); } diff --git a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp index 9dc476785c77..c11247c06b85 100644 --- a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp +++ b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp @@ -2290,7 +2290,8 @@ static void RemoveNonValidAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH, R.addAttribute(Attribute::NoAlias); if (!R.empty()) - AH.setAttributes(AH.getAttributes().removeAttributes(Ctx, Index, R)); + AH.setAttributes(AH.getAttributes().removeAttributes( + Ctx, Index, AttributeList::get(Ctx, Index, R))); } void diff --git a/unittests/IR/AttributesTest.cpp b/unittests/IR/AttributesTest.cpp index 0df7a847f8a5..7c3df2e19e8f 100644 --- a/unittests/IR/AttributesTest.cpp +++ b/unittests/IR/AttributesTest.cpp @@ -45,7 +45,7 @@ TEST(Attributes, Ordering) { AttributeList::get(C, 1, Attribute::SExt)}; AttributeList SetA = AttributeList::get(C, ASs); - AttributeList SetB = SetA.removeAttributes(C, 1, ASs[1].getAttributes(1)); + AttributeList SetB = SetA.removeAttributes(C, 1, ASs[1]); EXPECT_NE(SetA, SetB); }