Revert r301697 "[IR] Make add/remove Attributes use AttrBuilder inste…

…ad 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

@@ -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);
   }
 }
 

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

include/llvm/IR/Function.h

@@ -279,7 +279,7 @@ class Function : public GlobalObject, public ilist_node<Function> {
   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<Function> {
   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 {

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);

lib/IR/Attributes.cpp

@@ -936,17 +936,15 @@ 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,
                                           StringRef Kind,
                                           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,15 +1103,15 @@ 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
 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<unsigned> &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);

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);

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);
 }
 
 

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<BasicBlock>(VMap[ResumeEntry]);

lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

@@ -254,7 +254,7 @@ class DataFlowSanitizer : public ModulePass {
   MDNode *ColdCallWeights;
   DFSanABIList ABIList;
   DenseMap<Value *, Function *> 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();

lib/Transforms/Instrumentation/MemorySanitizer.cpp

@@ -2607,7 +2607,10 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
         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();
 }

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

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);
 }