Rationalise the attribute getter/setter methods on Function and CallSite.

Summary:
While woring on mapping attributes in the C API, it clearly appeared that the recent changes in the API on the C++ side left Function and Call/Invoke with an attribute API that grew in an ad hoc manner. This makes it difficult to work with it, because one doesn't know which overloads exists and which do not.

Make sure that getter/setter function exists for both enum and string version. Remove inconsistent getter/setter, unless they have many callsites.

This should make it easier to work with attributes in the future.

This doesn't change how attribute works.

Reviewers: bkramer, whitequark, mehdi_amini, void

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D21514

llvm-svn: 281019

7 files changed, 51 insertions, 97 deletions

diff --git a/llvm/include/llvm/CodeGen/CommandFlags.h b/llvm/include/llvm/CodeGen/CommandFlags.h
index 6821ec07a69..0eacf07c710 100644
--- a/llvm/include/llvm/CodeGen/CommandFlags.h
+++ b/llvm/include/llvm/CodeGen/CommandFlags.h
@@ -404,7 +404,8 @@ static inline void setFunctionAttributes(StringRef CPU, StringRef Features,
               if (F->getIntrinsicID() == Intrinsic::debugtrap ||
                   F->getIntrinsicID() == Intrinsic::trap)
                 Call->addAttribute(llvm::AttributeSet::FunctionIndex,
-                                   "trap-func-name", TrapFuncName);
+                                   Attribute::get(Ctx, "trap-func-name",
+                                                  TrapFuncName));
 
     // Let NewAttrs override Attrs.
     NewAttrs = Attrs.addAttributes(Ctx, AttributeSet::FunctionIndex, NewAttrs);
diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h
index 5ef03715b9a..f4da90bc902 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -338,6 +338,10 @@ public:
   /// may be faster.
   bool hasFnAttribute(Attribute::AttrKind Kind) const;
 
+  /// \brief Equivalent to hasAttribute(AttributeSet::FunctionIndex, Kind) but
+  /// may be faster.
+  bool hasFnAttribute(StringRef Kind) const;
+
   /// \brief Return true if the specified attribute is set for at least one
   /// parameter or for the return value. If Index is not nullptr, the index
   /// of a parameter with the specified attribute is provided.
diff --git a/llvm/include/llvm/IR/CallSite.h b/llvm/include/llvm/IR/CallSite.h
index 63b8be68841..e9d803a0ada 100644
--- a/llvm/include/llvm/IR/CallSite.h
+++ b/llvm/include/llvm/IR/CallSite.h
@@ -313,10 +313,10 @@ public:
 
   /// getAttributes/setAttributes - get or set the parameter attributes of
   /// the call.
-  const AttributeSet &getAttributes() const {
+  AttributeSet getAttributes() const {
     CALLSITE_DELEGATE_GETTER(getAttributes());
   }
-  void setAttributes(const AttributeSet &PAL) {
+  void setAttributes(AttributeSet PAL) {
     CALLSITE_DELEGATE_SETTER(setAttributes(PAL));
   }
 
@@ -324,10 +324,6 @@ public:
     CALLSITE_DELEGATE_SETTER(addAttribute(i, Kind));
   }
 
-  void addAttribute(unsigned i, StringRef Kind, StringRef Value) {
-    CALLSITE_DELEGATE_SETTER(addAttribute(i, Kind, Value));
-  }
-
   void addAttribute(unsigned i, Attribute Attr) {
     CALLSITE_DELEGATE_SETTER(addAttribute(i, Attr));
   }
@@ -340,10 +336,6 @@ public:
     CALLSITE_DELEGATE_SETTER(removeAttribute(i, Kind));
   }
 
-  void removeAttribute(unsigned i, Attribute Attr) {
-    CALLSITE_DELEGATE_SETTER(removeAttribute(i, Attr));
-  }
-
   /// \brief Return true if this function has the given attribute.
   bool hasFnAttr(Attribute::AttrKind Kind) const {
     CALLSITE_DELEGATE_GETTER(hasFnAttr(Kind));
diff --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h
index ff55fcbf7fe..129ba308004 100644
--- a/llvm/include/llvm/IR/Function.h
+++ b/llvm/include/llvm/IR/Function.h
@@ -164,27 +164,23 @@ public:
   void setAttributes(AttributeSet Attrs) { AttributeSets = Attrs; }
 
   /// @brief Add function attributes to this function.
-  void addFnAttr(Attribute::AttrKind N) {
-    setAttributes(AttributeSets.addAttribute(getContext(),
-                                             AttributeSet::FunctionIndex, N));
+  void addFnAttr(Attribute::AttrKind Kind) {
+    addAttribute(AttributeSet::FunctionIndex, Kind);
   }
 
-  /// @brief Remove function attributes from this function.
-  void removeFnAttr(Attribute::AttrKind Kind) {
-    setAttributes(AttributeSets.removeAttribute(
-        getContext(), AttributeSet::FunctionIndex, Kind));
+  /// @brief Add function attributes to this function.
+  void addFnAttr(StringRef Kind, StringRef Val = StringRef()) {
+    addAttribute(AttributeSet::FunctionIndex,
+                 Attribute::get(getContext(), Kind, Val));
   }
 
-  /// @brief Add function attributes to this function.
-  void addFnAttr(StringRef Kind) {
-    setAttributes(
-      AttributeSets.addAttribute(getContext(),
-                                 AttributeSet::FunctionIndex, Kind));
+  void addFnAttr(Attribute Attr) {
+    addAttribute(AttributeSet::FunctionIndex, Attr);
   }
-  void addFnAttr(StringRef Kind, StringRef Value) {
-    setAttributes(
-      AttributeSets.addAttribute(getContext(),
-                                 AttributeSet::FunctionIndex, Kind, Value));
+
+  /// @brief Remove function attributes from this function.
+  void removeFnAttr(Attribute::AttrKind Kind) {
+    removeAttribute(AttributeSet::FunctionIndex, Kind);
   }
 
   /// @brief Remove function attribute from this function.
@@ -204,7 +200,7 @@ public:
     return AttributeSets.hasFnAttribute(Kind);
   }
   bool hasFnAttribute(StringRef Kind) const {
-    return AttributeSets.hasAttribute(AttributeSet::FunctionIndex, Kind);
+    return AttributeSets.hasFnAttribute(Kind);
   }
 
   /// @brief Return the attribute for the given attribute kind.
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index da694f664d9..e68767b1943 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -1594,19 +1594,16 @@ public:
 
   /// Return the parameter attributes for this call.
   ///
-  const AttributeSet &getAttributes() const { return AttributeList; }
+  AttributeSet getAttributes() const { return AttributeList; }
 
   /// Set the parameter attributes for this call.
   ///
-  void setAttributes(const AttributeSet &Attrs) { AttributeList = Attrs; }
+  void setAttributes(AttributeSet Attrs) { AttributeList = Attrs; }
 
   /// adds the attribute to the list of attributes.
   void addAttribute(unsigned i, Attribute::AttrKind Kind);
 
   /// adds the attribute to the list of attributes.
-  void addAttribute(unsigned i, StringRef Kind, StringRef Value);
-
-  /// adds the attribute to the list of attributes.
   void addAttribute(unsigned i, Attribute Attr);
 
   /// removes the attribute from the list of attributes.
@@ -1615,9 +1612,6 @@ public:
   /// removes the attribute from the list of attributes.
   void removeAttribute(unsigned i, StringRef Kind);
 
-  /// removes the attribute from the list of attributes.
-  void removeAttribute(unsigned i, Attribute Attr);
-
   /// adds the dereferenceable attribute to the list of attributes.
   void addDereferenceableAttr(unsigned i, uint64_t Bytes);
 
@@ -1641,10 +1635,14 @@ public:
   bool paramHasAttr(unsigned i, Attribute::AttrKind Kind) const;
 
   /// Get the attribute of a given kind at a position.
-  Attribute getAttribute(unsigned i, Attribute::AttrKind Kind) const;
+  Attribute getAttribute(unsigned i, Attribute::AttrKind Kind) const {
+    return getAttributes().getAttribute(i, Kind);
+  }
 
   /// Get the attribute of a given kind at a position.
-  Attribute getAttribute(unsigned i, StringRef Kind) const;
+  Attribute getAttribute(unsigned i, StringRef Kind) const {
+    return getAttributes().getAttribute(i, Kind);
+  }
 
   /// Return true if the data operand at index \p i has the attribute \p
   /// A.
@@ -1763,8 +1761,7 @@ public:
     addAttribute(AttributeSet::FunctionIndex, Attribute::Convergent);
   }
   void setNotConvergent() {
-    removeAttribute(AttributeSet::FunctionIndex,
-                    Attribute::get(getContext(), Attribute::Convergent));
+    removeAttribute(AttributeSet::FunctionIndex, Attribute::Convergent);
   }
 
   /// Determine if the call returns a structure through first
@@ -1821,17 +1818,17 @@ public:
   }
 
 private:
-  template <typename AttrKind> bool hasFnAttrImpl(AttrKind A) const {
-    if (AttributeList.hasAttribute(AttributeSet::FunctionIndex, A))
+  template <typename AttrKind> bool hasFnAttrImpl(AttrKind Kind) const {
+    if (AttributeList.hasAttribute(AttributeSet::FunctionIndex, Kind))
       return true;
 
     // Operand bundles override attributes on the called function, but don't
     // override attributes directly present on the call instruction.
-    if (isFnAttrDisallowedByOpBundle(A))
+    if (isFnAttrDisallowedByOpBundle(Kind))
       return false;
 
     if (const Function *F = getCalledFunction())
-      return F->getAttributes().hasAttribute(AttributeSet::FunctionIndex, A);
+      return F->getAttributes().hasAttribute(AttributeSet::FunctionIndex, Kind);
     return false;
   }
 
@@ -3567,11 +3564,11 @@ public:
 
   /// Return the parameter attributes for this invoke.
   ///
-  const AttributeSet &getAttributes() const { return AttributeList; }
+  AttributeSet getAttributes() const { return AttributeList; }
 
   /// Set the parameter attributes for this invoke.
   ///
-  void setAttributes(const AttributeSet &Attrs) { AttributeList = Attrs; }
+  void setAttributes(AttributeSet Attrs) { AttributeList = Attrs; }
 
   /// adds the attribute to the list of attributes.
   void addAttribute(unsigned i, Attribute::AttrKind Kind);
@@ -3585,9 +3582,6 @@ public:
   /// removes the attribute from the list of attributes.
   void removeAttribute(unsigned i, StringRef Kind);
 
-  /// removes the attribute from the list of attributes.
-  void removeAttribute(unsigned i, Attribute Attr);
-
   /// adds the dereferenceable attribute to the list of attributes.
   void addDereferenceableAttr(unsigned i, uint64_t Bytes);
 
@@ -3611,10 +3605,14 @@ public:
   bool paramHasAttr(unsigned i, Attribute::AttrKind Kind) const;
 
   /// Get the attribute of a given kind at a position.
-  Attribute getAttribute(unsigned i, Attribute::AttrKind Kind) const;
+  Attribute getAttribute(unsigned i, Attribute::AttrKind Kind) const {
+    return getAttributes().getAttribute(i, Kind);
+  }
 
   /// Get the attribute of a given kind at a position.
-  Attribute getAttribute(unsigned i, StringRef Kind) const;
+  Attribute getAttribute(unsigned i, StringRef Kind) const {
+    return getAttributes().getAttribute(i, Kind);
+  }
 
   /// Return true if the data operand at index \p i has the attribute \p
   /// A.
@@ -3728,8 +3726,7 @@ public:
     addAttribute(AttributeSet::FunctionIndex, Attribute::Convergent);
   }
   void setNotConvergent() {
-    removeAttribute(AttributeSet::FunctionIndex,
-                    Attribute::get(getContext(), Attribute::Convergent));
+    removeAttribute(AttributeSet::FunctionIndex, Attribute::Convergent);
   }
 
   /// Determine if the call returns a structure through first
@@ -3815,17 +3812,17 @@ private:
   unsigned getNumSuccessorsV() const override;
   void setSuccessorV(unsigned idx, BasicBlock *B) override;
 
-  template <typename AttrKind> bool hasFnAttrImpl(AttrKind A) const {
-    if (AttributeList.hasAttribute(AttributeSet::FunctionIndex, A))
+  template <typename AttrKind> bool hasFnAttrImpl(AttrKind Kind) const {
+    if (AttributeList.hasAttribute(AttributeSet::FunctionIndex, Kind))
       return true;
 
     // Operand bundles override attributes on the called function, but don't
     // override attributes directly present on the invoke instruction.
-    if (isFnAttrDisallowedByOpBundle(A))
+    if (isFnAttrDisallowedByOpBundle(Kind))
       return false;
 
     if (const Function *F = getCalledFunction())
-      return F->getAttributes().hasAttribute(AttributeSet::FunctionIndex, A);
+      return F->getAttributes().hasAttribute(AttributeSet::FunctionIndex, Kind);
     return false;
   }
 
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index 2b76de77dca..b8b9df7b0ce 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -1115,6 +1115,10 @@ bool AttributeSet::hasFnAttribute(Attribute::AttrKind Kind) const {
   return pImpl && pImpl->hasFnAttribute(Kind);
 }
 
+bool AttributeSet::hasFnAttribute(StringRef Kind) const {
+  return hasAttribute(AttributeSet::FunctionIndex, Kind);
+}
+
 bool AttributeSet::hasAttrSomewhere(Attribute::AttrKind Attr,
                                     unsigned *Index) const {
   if (!pImpl) return false;
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index 89c2caf0dbb..e29f8250f12 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -350,12 +350,6 @@ void CallInst::addAttribute(unsigned i, Attribute::AttrKind Kind) {
   setAttributes(PAL);
 }
 
-void CallInst::addAttribute(unsigned i, StringRef Kind, StringRef Value) {
-  AttributeSet PAL = getAttributes();
-  PAL = PAL.addAttribute(getContext(), i, Kind, Value);
-  setAttributes(PAL);
-}
-
 void CallInst::addAttribute(unsigned i, Attribute Attr) {
   AttributeSet PAL = getAttributes();
   PAL = PAL.addAttribute(getContext(), i, Attr);
@@ -374,15 +368,6 @@ void CallInst::removeAttribute(unsigned i, StringRef Kind) {
   setAttributes(PAL);
 }
 
-void CallInst::removeAttribute(unsigned i, Attribute Attr) {
-  AttributeSet PAL = getAttributes();
-  AttrBuilder B(Attr);
-  LLVMContext &Context = getContext();
-  PAL = PAL.removeAttributes(Context, i,
-                             AttributeSet::get(Context, i, B));
-  setAttributes(PAL);
-}
-
 void CallInst::addDereferenceableAttr(unsigned i, uint64_t Bytes) {
   AttributeSet PAL = getAttributes();
   PAL = PAL.addDereferenceableAttr(getContext(), i, Bytes);
@@ -405,14 +390,6 @@ bool CallInst::paramHasAttr(unsigned i, Attribute::AttrKind Kind) const {
   return false;
 }
 
-Attribute CallInst::getAttribute(unsigned i, Attribute::AttrKind Kind) const {
-  return getAttributes().getAttribute(i, Kind);
-}
-
-Attribute CallInst::getAttribute(unsigned i, StringRef Kind) const {
-  return getAttributes().getAttribute(i, Kind);
-}
-
 bool CallInst::dataOperandHasImpliedAttr(unsigned i,
                                          Attribute::AttrKind Kind) const {
   // There are getNumOperands() - 1 data operands.  The last operand is the
@@ -766,23 +743,6 @@ void InvokeInst::removeAttribute(unsigned i, StringRef Kind) {
   setAttributes(PAL);
 }
 
-void InvokeInst::removeAttribute(unsigned i, Attribute Attr) {
-  AttributeSet PAL = getAttributes();
-  AttrBuilder B(Attr);
-  PAL = PAL.removeAttributes(getContext(), i,
-                             AttributeSet::get(getContext(), i, B));
-  setAttributes(PAL);
-}
-
-Attribute InvokeInst::getAttribute(unsigned i,
-                                   Attribute::AttrKind Kind) const {
-  return getAttributes().getAttribute(i, Kind);
-}
-
-Attribute InvokeInst::getAttribute(unsigned i, StringRef Kind) const {
-  return getAttributes().getAttribute(i, Kind);
-}
-
 void InvokeInst::addDereferenceableAttr(unsigned i, uint64_t Bytes) {
   AttributeSet PAL = getAttributes();
   PAL = PAL.addDereferenceableAttr(getContext(), i, Bytes);