Avoid using AttrBuilder as an intermediate step when not needed

[serge-sans-paille]

Summary

AttrBuilder is an helpful but costly data structure; it is not
needed to construct one when a single element is added.

Using an explicit constructor for AttrBuilder helps spotting cases where an
implicit conversion happens when the appropriate method can be used instead.

[serge-sans-paille]

rebase on main branch

[serge-sans-paille]

Mandatory compile-time tracker entry: https://llvm-compile-time-tracker.com/compare.php?from=35cca45b09b87cc31cda35a66fc93bf4d630f8d2&to=1a00df8c45dd9949ba4cd1bdee184d0ad9ca45af&stat=instructions some green entries are always good ;-)

[nikic]

I'm not a fan of this change, because it duplicates logic and introduces a good number of additional overloads, for not a lot of practical benefit on compile-time. I think our general goal should be to make AttrBuilder fast enough that we do not need to add specialized implementations for specific implementations.

If we change AttrBuilder to just be an API around SmallVector<Attribute> we get some nice improvements: https://llvm-compile-time-tracker.com/compare.php?from=64590312d4b84e13345a98b9d7d7bbc4e2b0c166&to=1ba9059e40345e3530dcb0af9b6bda049f4aab8f&stat=instructions At that point, doing an unnecessary AttributeSet -> AttrBuilder conversion only involves copying attributes into a different vector, which is cheap enough that it probably does not warrant further optimization anymore. I also checked what happens if I combine that with the optimized merge implementation you added here, but it doesn't seem to make a measurable difference (https://llvm-compile-time-tracker.com/compare.php?from=1ba9059e40345e3530dcb0af9b6bda049f4aab8f&to=6890ee94894d93b744514a73e8b592e8284a5f2f&stat=instructions). My guess is that the right hand side AttrBuilder tends to only contain very few attributes, so doing individual lower_bound insertions works well enough.

Unfortunately, I discovered a problem while looking into this: Apparently, we don't have a well-defined behavior when adding an attribute that already exists (same key, different value). We have a mix of different behaviors:

• String attributes take their value from the RHS when merging.
• Int and type attributes take their value from the LHS when merging (https://github.com/llvm/llvm-project/blob/3ba96cb2c94904ab68a59cfd2391846f1e9b9902/llvm/lib/IR/Attributes.cpp#L1760-L1767).
• We have one place that asserts that overwriting with different alignment is not allowed, but other places that don't assert this (https://github.com/llvm/llvm-project/blob/3ba96cb2c94904ab68a59cfd2391846f1e9b9902/llvm/lib/IR/Attributes.cpp#L1253).
• And then to add insult to injury, AttributeSet::addAttributes() performs the merging the wrong way around (https://github.com/llvm/llvm-project/blob/3ba96cb2c94904ab68a59cfd2391846f1e9b9902/llvm/lib/IR/Attributes.cpp#L630-L632), so the behavior is the opposite from what AttributeList::addAttributesAtIndex() (the more commonly used API) does.

I think before we optimize attribute addition, we first need to decide on a single behavior. I think it would either make sense to pick the one you implemented here (i.e. attribute from RHS wins) or to forbid adding an attribute with the same key (unless it also has same value or no value). In the latter case the user would be required to remove the old attribute beforehand.

llvm/include/llvm/IR/Attributes.h

private:
   explicit AttributeSet(AttributeSetNode *ASN) : SetNode(ASN) {}
 
 public:
   /// AttributeSet is a trivially copyable value type.
   AttributeSet() = default;
   AttributeSet(const AttributeSet &) = default;
   ~AttributeSet() = default;
 
+  static AttributeSet get(LLVMContext &C, Attribute A);
   static AttributeSet get(LLVMContext &C, const AttrBuilder &B);
   static AttributeSet get(LLVMContext &C, ArrayRef<Attribute> Attrs);
 
   bool operator==(const AttributeSet &O) const { return SetNode == O.SetNode; }
   bool operator!=(const AttributeSet &O) const { return !(*this == O); }
 
   /// Add an argument attribute. Returns a new set because attribute sets are
   /// immutable.
LLVM_NODISCARD AttributeList addAttributeAtIndex(LLVMContext &C,
                                                    unsigned Index,
                                                    Attribute A) const;
 
   /// Add attributes to the attribute set at the given index.
   /// Returns a new list because attribute lists are immutable.
   LLVM_NODISCARD AttributeList addAttributesAtIndex(LLVMContext &C,
                                                     unsigned Index,
                                                     const AttrBuilder &B) const;
+  LLVM_NODISCARD AttributeList addAttributesAtIndex(LLVMContext &C,
+                                                    unsigned Index,
+                                                    AttributeSet AS) const;
 
   /// Add a function attribute to the list. Returns a new list because
   /// attribute lists are immutable.
   LLVM_NODISCARD AttributeList addFnAttribute(LLVMContext &C,
                                               Attribute::AttrKind Kind) const {
     return addAttributeAtIndex(C, FunctionIndex, Kind);
   }
 
public:
 
   /// Add function attribute to the list. Returns a new list because
   /// attribute lists are immutable.
   LLVM_NODISCARD AttributeList addFnAttributes(LLVMContext &C,
                                                const AttrBuilder &B) const {
     return addAttributesAtIndex(C, FunctionIndex, B);
   }
 
+  LLVM_NODISCARD AttributeList addFnAttributes(LLVMContext &C,
+                                               AttributeSet AS) const {
+    return addAttributesAtIndex(C, FunctionIndex, AS);
+  }
+
   /// Add a return value attribute to the list. Returns a new list because
   /// attribute lists are immutable.
   LLVM_NODISCARD AttributeList addRetAttribute(LLVMContext &C,
                                                Attribute::AttrKind Kind) const {
     return addAttributeAtIndex(C, ReturnIndex, Kind);
   }
 
   /// Add a return value attribute to the list. Returns a new list because
   /// attribute lists are immutable.
   LLVM_NODISCARD AttributeList addRetAttribute(LLVMContext &C,
                                                Attribute Attr) const {
     return addAttributeAtIndex(C, ReturnIndex, Attr);
   }
 
   /// Add a return value attribute to the list. Returns a new list because
   /// attribute lists are immutable.
   LLVM_NODISCARD AttributeList addRetAttributes(LLVMContext &C,
                                                 const AttrBuilder &B) const {
     return addAttributesAtIndex(C, ReturnIndex, B);
   }
+  LLVM_NODISCARD AttributeList addRetAttributes(LLVMContext &C,
+                                                AttributeSet AS) const {
+    return addAttributesAtIndex(C, ReturnIndex, AS);
+  }
 
   /// Add an argument attribute to the list. Returns a new list because
   /// attribute lists are immutable.
   LLVM_NODISCARD AttributeList addParamAttribute(
       LLVMContext &C, unsigned ArgNo, Attribute::AttrKind Kind) const {
     return addAttributeAtIndex(C, ArgNo + FirstArgIndex, Kind);
   }
 
public:
 
   /// Add an argument attribute to the list. Returns a new list because
   /// attribute lists are immutable.
   LLVM_NODISCARD AttributeList addParamAttributes(LLVMContext &C,
                                                   unsigned ArgNo,
                                                   const AttrBuilder &B) const {
     return addAttributesAtIndex(C, ArgNo + FirstArgIndex, B);
   }
+  LLVM_NODISCARD AttributeList addParamAttributes(LLVMContext &C,
+                                                  unsigned ArgNo,
+                                                  AttributeSet AS) const {
+    return addAttributesAtIndex(C, ArgNo + FirstArgIndex, AS);
+  }
 
   /// Remove the specified attribute at the specified index from this
   /// attribute list. Returns a new list because attribute lists are immutable.
   LLVM_NODISCARD AttributeList removeAttributeAtIndex(
       LLVMContext &C, unsigned Index, Attribute::AttrKind Kind) const;
 
   /// Remove the specified attribute at the specified index from this
   /// attribute list. Returns a new list because attribute lists are immutable.
class AttrBuilder {
   Optional<unsigned> kindToIntIndex(Attribute::AttrKind Kind) const;
   Optional<unsigned> kindToTypeIndex(Attribute::AttrKind Kind) const;
 
 public:
   AttrBuilder() = default;
   AttrBuilder(const AttrBuilder &) = delete;
   AttrBuilder(AttrBuilder &&) = default;
 
-  AttrBuilder(const Attribute &A) {
-    addAttribute(A);
-  }
+  explicit AttrBuilder(const Attribute &A) { addAttribute(A); }
+
+  explicit AttrBuilder(AttributeSet AS);
 
   AttrBuilder(AttributeList AS, unsigned Idx);
-  AttrBuilder(AttributeSet AS);
 
   void clear();
 
   /// Add an attribute to the builder.
   AttrBuilder &addAttribute(Attribute::AttrKind Val) {
     assert((unsigned)Val < Attribute::EndAttrKinds &&
            "Attribute out of range!");
     assert(Attribute::isEnumAttrKind(Val) &&
public:
   AttrBuilder &addAttribute(StringRef A, StringRef V = StringRef());
 
   /// Remove an attribute from the builder.
   AttrBuilder &removeAttribute(Attribute::AttrKind Val);
 
   /// Remove the attributes from the builder.
   AttrBuilder &removeAttributes(AttributeList A, uint64_t WithoutIndex);
 
+  /// Remove the attributes from the builder.
+  AttrBuilder &removeAttributes(AttributeSet AS);
+
   /// Remove the target-dependent attribute to the builder.
   AttrBuilder &removeAttribute(StringRef A);
 
   /// Add the attributes from the builder.
   AttrBuilder &merge(const AttrBuilder &B);
 
+  /// Add the attributes from the builder.
+  AttrBuilder &merge(AttributeSet AS);
+
+  /// Add the attributes from the builder.
+  AttrBuilder &merge(Attribute A);
+
   /// Remove the attributes from the builder.
   AttrBuilder &remove(const AttrBuilder &B);
 
+  /// Remove the attribute from the builder.
+  AttrBuilder &remove(Attribute A);
+
   /// Return true if the builder has any attribute that's in the
   /// specified builder.
   bool overlaps(const AttrBuilder &B) const;
 
   /// Return true if the builder has the specified attribute.
   bool contains(Attribute::AttrKind A) const {
     assert((unsigned)A < Attribute::EndAttrKinds && "Attribute out of range!");
     return Attrs[A];

llvm/lib/IR/AttributeImpl.h

class AttributeSetNode final
 
   unsigned NumAttrs; ///< Number of attributes in this node.
   AttributeBitSet AvailableAttrs; ///< Available enum attributes.
 
   DenseMap<StringRef, Attribute> StringAttrs;
 
   AttributeSetNode(ArrayRef<Attribute> Attrs);
 
-  static AttributeSetNode *getSorted(LLVMContext &C,
-                                     ArrayRef<Attribute> SortedAttrs);
   Optional<Attribute> findEnumAttribute(Attribute::AttrKind Kind) const;
 
 public:
   // AttributesSetNode is uniqued, these should not be available.
   AttributeSetNode(const AttributeSetNode &) = delete;
   AttributeSetNode &operator=(const AttributeSetNode &) = delete;
 
   void operator delete(void *p) { ::operator delete(p); }
 
+  static AttributeSetNode *get(LLVMContext &C, Attribute A);
+
   static AttributeSetNode *get(LLVMContext &C, const AttrBuilder &B);
 
   static AttributeSetNode *get(LLVMContext &C, ArrayRef<Attribute> Attrs);
 
+  static AttributeSetNode *getSorted(LLVMContext &C,
+                                     ArrayRef<Attribute> SortedAttrs);
+
   /// Return the number of attributes this AttributeList contains.
   unsigned getNumAttributes() const { return NumAttrs; }
 
   bool hasAttribute(Attribute::AttrKind Kind) const {
     return AvailableAttrs.hasAttribute(Kind);
   }
   bool hasAttribute(StringRef Kind) const;
   bool hasAttributes() const { return NumAttrs != 0; }

llvm/lib/IR/Attributes.cpp

if (getKindAsString() == AI.getKindAsString())
     return getValueAsString() < AI.getValueAsString();
   return getKindAsString() < AI.getKindAsString();
 }
 
 //===----------------------------------------------------------------------===//
 // AttributeSet Definition
 //===----------------------------------------------------------------------===//
 
+AttributeSet AttributeSet::get(LLVMContext &C, Attribute A) {
+  return AttributeSet(AttributeSetNode::get(C, A));
+}
+
 AttributeSet AttributeSet::get(LLVMContext &C, const AttrBuilder &B) {
   return AttributeSet(AttributeSetNode::get(C, B));
 }
 
 AttributeSet AttributeSet::get(LLVMContext &C, ArrayRef<Attribute> Attrs) {
   return AttributeSet(AttributeSetNode::get(C, Attrs));
 }
 
 AttributeSet AttributeSet::addAttribute(LLVMContext &C,
                                         Attribute::AttrKind Kind) const {
   if (hasAttribute(Kind)) return *this;
-  AttrBuilder B;
-  B.addAttribute(Kind);
-  return addAttributes(C, AttributeSet::get(C, B));
+  return addAttributes(C, AttributeSet::get(C, Attribute::get(C, Kind)));
 }
 
 AttributeSet AttributeSet::addAttribute(LLVMContext &C, StringRef Kind,
                                         StringRef Value) const {
-  AttrBuilder B;
-  B.addAttribute(Kind, Value);
-  return addAttributes(C, AttributeSet::get(C, B));
+  return addAttributes(C, AttributeSet::get(C, Attribute::get(C, Kind, Value)));
 }
 
 AttributeSet AttributeSet::addAttributes(LLVMContext &C,
                                          const AttributeSet AS) const {
   if (!hasAttributes())
     return AS;
 
   if (!AS.hasAttributes())
     return *this;
 
-  AttrBuilder B(AS);
-  for (const auto &I : *this)
-    B.addAttribute(I);
+  // It is more efficient to merge the two sorted Attribute array we have at
+  // that point rather than creating an AttrBuilder to to the merge.
 
- return get(C, B);
+  SmallVector<Attribute, 8> SortedAttrs;
+  auto SelfIter = begin(), SelfEnd = end();
+  auto OtherIter = AS.begin(), OtherEnd = AS.end();
+
+  // Comparator that only compares key and treat Attribute with the same key but
+  // different value as equivalent.
+  auto Cmp = [](Attribute A0, Attribute A1) {
+    if (A0.isStringAttribute()) {
+      if (A1.isStringAttribute())
+        return strcmp(A0.getKindAsString().data(), A1.getKindAsString().data());
+      else
+        return +1;
+    } else {
+      if (A1.isStringAttribute())
+        return -1;
+      else
+        return (int)A0.getKindAsEnum() - (int)A1.getKindAsEnum();
+    }
+  };
+
+  // This is close to std::set_union + std::unique in a row.
+  while (OtherIter < OtherEnd && SelfIter < SelfEnd) {
+    int C = Cmp(*OtherIter, *SelfIter);
+    if (C < 0) {
+      SortedAttrs.push_back(*OtherIter);
+      ++OtherIter;
+    } else if (C == 0) {
+      // In case of equality, the new version is honored.
+      SortedAttrs.push_back(*OtherIter);
+      ++OtherIter;
+      ++SelfIter;
+    } else {
+      SortedAttrs.push_back(*SelfIter);
+      ++SelfIter;
+    }
+  }
+  if (OtherIter < OtherEnd)
+    SortedAttrs.append(OtherIter, OtherEnd);
+  else
+    SortedAttrs.append(SelfIter, SelfEnd);
+
+  return AttributeSet(AttributeSetNode::getSorted(C, SortedAttrs));
 }
 
 AttributeSet AttributeSet::removeAttribute(LLVMContext &C,
                                              Attribute::AttrKind Kind) const {
   if (!hasAttribute(Kind)) return *this;
   AttrBuilder B(*this);
   B.removeAttribute(Kind);
   return get(C, B);
dbgs() << "AS =\n";
     dbgs() << "  { ";
     dbgs() << getAsString(true) << " }\n";
 }
 #endif
 
 //===----------------------------------------------------------------------===//
 // AttributeSetNode Definition
 //===----------------------------------------------------------------------===//
+AttributeSetNode *AttributeSetNode::get(LLVMContext &C, Attribute A) {
+  return getSorted(C, {A});
+}
 
 AttributeSetNode::AttributeSetNode(ArrayRef<Attribute> Attrs)
     : NumAttrs(Attrs.size()) {
   // There's memory after the node where we can store the entries in.
   llvm::copy(Attrs, getTrailingObjects<Attribute>());
 
   for (const auto &I : *this) {
     if (I.isStringAttribute())
AttributeList AttributeList::get(LLVMContext &C,
   return getImpl(C, NewAttrSets);
 }
 
 AttributeList
 AttributeList::addAttributeAtIndex(LLVMContext &C, unsigned Index,
                                    Attribute::AttrKind Kind) const {
   if (hasAttributeAtIndex(Index, Kind))
     return *this;
+
   AttributeSet Attrs = getAttributes(Index);
-  // TODO: Insert at correct position and avoid sort.
-  SmallVector<Attribute, 8> NewAttrs(Attrs.begin(), Attrs.end());
-  NewAttrs.push_back(Attribute::get(C, Kind));
-  return setAttributesAtIndex(C, Index, AttributeSet::get(C, NewAttrs));
+  return setAttributesAtIndex(C, Index, Attrs.addAttribute(C, Kind));
 }
 
 AttributeList AttributeList::addAttributeAtIndex(LLVMContext &C, unsigned Index,
                                                  StringRef Kind,
                                                  StringRef Value) const {
-  AttrBuilder B;
-  B.addAttribute(Kind, Value);
-  return addAttributesAtIndex(C, Index, B);
+  return addAttributeAtIndex(C, Index, Attribute::get(C, Kind, Value));
 }
 
 AttributeList AttributeList::addAttributeAtIndex(LLVMContext &C, unsigned Index,
                                                  Attribute A) const {
-  AttrBuilder B;
-  B.addAttribute(A);
-  return addAttributesAtIndex(C, Index, B);
+  AttrBuilder B(getAttributes(Index));
+  B.merge(A);
+  getAttributes(Index);
+  return setAttributesAtIndex(C, Index, AttributeSet::get(C, B));
 }
 
 AttributeList AttributeList::setAttributesAtIndex(LLVMContext &C,
                                                   unsigned Index,
                                                   AttributeSet Attrs) const {
   Index = attrIdxToArrayIdx(Index);
   SmallVector<AttributeSet, 4> AttrSets(this->begin(), this->end());
   if (Index >= AttrSets.size())
assert((!OldAlign || !NewAlign || OldAlign == NewAlign) &&
          "Attempt to change alignment!");
 #endif
 
   AttrBuilder Merged(getAttributes(Index));
   Merged.merge(B);
   return setAttributesAtIndex(C, Index, AttributeSet::get(C, Merged));
 }
 
+AttributeList AttributeList::addAttributesAtIndex(LLVMContext &C,
+                                                  unsigned Index,
+                                                  AttributeSet AS) const {
+  if (!AS.hasAttributes())
+    return *this;
+
+  if (!pImpl)
+    return AttributeList::get(C, {{Index, AS}});
+
+#ifndef NDEBUG
+  // FIXME it is not obvious how this should work for alignment. For now, say
+  // we can't change a known alignment.
+  const MaybeAlign OldAlign = getAttributes(Index).getAlignment();
+  const MaybeAlign NewAlign = AS.getAlignment();
+  assert((!OldAlign || !NewAlign || OldAlign == NewAlign) &&
+         "Attempt to change alignment!");
+#endif
+
+  AttrBuilder Merged(getAttributes(Index));
+  Merged.merge(AS);
+  return setAttributesAtIndex(C, Index, AttributeSet::get(C, Merged));
+}
+
 AttributeList AttributeList::addParamAttribute(LLVMContext &C,
                                                ArrayRef<unsigned> ArgNos,
                                                Attribute A) const {
   assert(llvm::is_sorted(ArgNos));
 
   SmallVector<AttributeSet, 4> AttrSets(this->begin(), this->end());
   unsigned MaxIndex = attrIdxToArrayIdx(ArgNos.back() + FirstArgIndex);
   if (MaxIndex >= AttrSets.size())
if (Optional<unsigned> TypeIndex = kindToTypeIndex(Val))
     TypeAttrs[*TypeIndex] = nullptr;
   else if (Optional<unsigned> IntIndex = kindToIntIndex(Val))
     IntAttrs[*IntIndex] = 0;
 
   return *this;
 }
 
 AttrBuilder &AttrBuilder::removeAttributes(AttributeList A, uint64_t Index) {
-  remove(A.getAttributes(Index));
+  removeAttributes(A.getAttributes(Index));
+  return *this;
+}
+
+AttrBuilder &AttrBuilder::removeAttributes(AttributeSet AS) {
+  for (Attribute A : AS)
+    remove(A);
   return *this;
 }
 
 AttrBuilder &AttrBuilder::removeAttribute(StringRef A) {
   TargetDepAttrs.erase(A);
   return *this;
 }
 
AttrBuilder &AttrBuilder::merge(const AttrBuilder &B) {
   Attrs |= B.Attrs;
 
   for (const auto &I : B.td_attrs())
     TargetDepAttrs[I.first] = I.second;
 
   return *this;
 }
 
+AttrBuilder &AttrBuilder::merge(AttributeSet AS) {
+  for (Attribute A : AS)
+    merge(A);
+  return *this;
+}
+
+AttrBuilder &AttrBuilder::merge(Attribute A) {
+  if (A.isStringAttribute()) {
+    TargetDepAttrs[A.getKindAsString()] = A.getValueAsString();
+  } else {
+    auto Kind = A.getKindAsEnum();
+    if (!Attrs[Kind]) {
+      Attrs[Kind] = true;
+      if (A.isIntAttribute()) {
+        IntAttrs[*kindToIntIndex(Kind)] = A.getValueAsInt();
+      } else if (A.isTypeAttribute()) {
+        TypeAttrs[*kindToTypeIndex(Kind)] = A.getValueAsType();
+      }
+    }
+  }
+  return *this;
+}
+
 AttrBuilder &AttrBuilder::remove(const AttrBuilder &B) {
   // FIXME: What if both have an int/type attribute, but they don't match?!
   for (unsigned Index = 0; Index < Attribute::NumIntAttrKinds; ++Index)
     if (B.IntAttrs[Index])
       IntAttrs[Index] = 0;
 
   for (unsigned Index = 0; Index < Attribute::NumTypeAttrKinds; ++Index)
     if (B.TypeAttrs[Index])
       TypeAttrs[Index] = nullptr;
 
   Attrs &= ~B.Attrs;
 
   for (const auto &I : B.td_attrs())
     TargetDepAttrs.erase(I.first);
 
   return *this;
 }
 
+AttrBuilder &AttrBuilder::remove(Attribute A) {
+  if (A.isStringAttribute())
+    return removeAttribute(A.getKindAsString());
+  else
+    return removeAttribute(A.getKindAsEnum());
+}
+
 bool AttrBuilder::overlaps(const AttrBuilder &B) const {
   // First check if any of the target independent attributes overlap.
   if ((Attrs & B.Attrs).any())
     return true;
 
   // Then check if any target dependent ones do.
   for (const auto &I : td_attrs())
     if (B.contains(I.first))

llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

 // Create new attribute set containing only attributes which can be transferred
 // from original call to the safepoint.
 static AttributeList legalizeCallAttributes(LLVMContext &Ctx,
                                             AttributeList AL) {
   if (AL.isEmpty())
     return AL;
 
   // Remove the readonly, readnone, and statepoint function attributes.
-  AttrBuilder FnAttrs = AL.getFnAttrs();
+  AttrBuilder FnAttrs(AL.getFnAttrs());
   for (auto Attr : FnAttrsToStrip)
     FnAttrs.removeAttribute(Attr);
 
   for (Attribute A : AL.getFnAttrs()) {
     if (isStatepointDirectiveAttr(A))
       FnAttrs.remove(A);
   }
 
   // Just skip parameter and return attributes for now
-  return AttributeList::get(Ctx, AttributeList::FunctionIndex,
-                            AttributeSet::get(Ctx, FnAttrs));
+  return AttributeList::get(Ctx, AttributeList::FunctionIndex, FnAttrs);
 }
 
 /// Helper function to place all gc relocates necessary for the given
 /// statepoint.
 /// Inputs:
 ///   liveVariables - list of variables to be relocated.
 ///   basePtrs - base pointers.
 ///   statepointToken - statepoint instruction to which relocates should be

llvm/unittests/IR/AttributesTest.cpp

TEST(Attributes, Ordering) {
   EXPECT_TRUE(ByVal < Align4);
   EXPECT_FALSE(ByVal < ByVal);
 
   AttributeList ASs[] = {AttributeList::get(C, 2, Attribute::ZExt),
                          AttributeList::get(C, 1, Attribute::SExt)};
 
   AttributeList SetA = AttributeList::get(C, ASs);
   AttributeList SetB =
-      SetA.removeParamAttributes(C, 0, ASs[1].getParamAttrs(0));
+      SetA.removeParamAttributes(C, 0, AttrBuilder(ASs[1].getParamAttrs(0)));
   EXPECT_NE(SetA, SetB);
 }
 
 TEST(Attributes, AddAttributes) {
   LLVMContext C;
   AttributeList AL;
   AttrBuilder B;
   B.addAttribute(Attribute::NoReturn);