[NFC][IR] Force accesses to Function attributes go through getters/setters

[vporpo]

Summary

This patch moves Function::AttributeList to a separate FnAttributeList class
where it is declared as private. The Function class inherits from
FnAttributeList but all accesses to the attributes have to go through
getter/setter functions.

[arsenm]

What's the point of doing this? The attributes are already private

[vporpo]

It funnels all changes to attributes through a single function, which makes it really easy to track all attribute changes from a single point.

[nikic]

In D144040#4127140, @vporpo wrote:

It funnels all changes to attributes through a single function, which makes it really easy to track all attribute changes from a single point.

And what is this needed for / what will this be used for?

[vporpo]

And what is this needed for / what will this be used for?

This is to make it easier to track changes to the attributes for Checkpointing, but regardless I think this is generally useful e.g., for debugging, as you can watch all changes from a single point.

[aeubanks]

(RFC link https://discourse.llvm.org/t/rfc-lightweight-llvm-ir-checkpointing/68446)

[nikic]

I see. In that case, I think this change makes sense as part of a patch stack to implement that RFC (if it is accepted), but I don't think we should do this otherwise.

llvm/include/llvm/IR/Function.h

 class LLVMContext;
 class Module;
 class raw_ostream;
 class Type;
 class User;
 class BranchProbabilityInfo;
 class BlockFrequencyInfo;
 
+/// Helper class that forces all accesses to the function parameter attributes
+/// go through a set/get interface.
+class FnAttributeList {
+  /// Parameter attributes.
+  AttributeList Attrs;
+
+public:
+  void setAttributes(AttributeList NewAttrs);
+  const AttributeList &getAttributes() const { return Attrs; }
+};
+
 class LLVM_EXTERNAL_VISIBILITY Function : public GlobalObject,
-                                          public ilist_node<Function> {
+                                          public ilist_node<Function>,
+                                          public FnAttributeList {
+  friend class AttributeListSetGet;
 public:
   using BasicBlockListType = SymbolTableList<BasicBlock>;
 
   // BasicBlock iterators...
   using iterator = BasicBlockListType::iterator;
   using const_iterator = BasicBlockListType::const_iterator;
 
   using arg_iterator = Argument *;
   using const_arg_iterator = const Argument *;
 
 private:
   // Important things that make up a function!
   BasicBlockListType BasicBlocks;         ///< The basic blocks
   mutable Argument *Arguments = nullptr;  ///< The formal arguments
   size_t NumArgs;
   std::unique_ptr<ValueSymbolTable>
       SymTab;                             ///< Symbol table of args/instructions
-  AttributeList AttributeSets;            ///< Parameter attributes
 
   /*
    * Value::SubclassData
    *
    * bit 0      : HasLazyArguments
    * bit 1      : HasPrefixData
    * bit 2      : HasPrologueData
    * bit 3      : HasPersonalityFn
public:
   ///                             to use during code generation.
   bool hasGC() const {
     return getSubclassDataFromValue() & (1<<14);
   }
   const std::string &getGC() const;
   void setGC(std::string Str);
   void clearGC();
 
-  /// Return the attribute list for this Function.
-  AttributeList getAttributes() const { return AttributeSets; }
-
-  /// Set the attribute list for this Function.
-  void setAttributes(AttributeList Attrs) { AttributeSets = Attrs; }
-
   // TODO: remove non-AtIndex versions of these methods.
   /// adds the attribute to the list of attributes.
   void addAttributeAtIndex(unsigned i, Attribute Attr);
 
   /// Add function attributes to this function.
   void addFnAttr(Attribute::AttrKind Kind);
 
   /// Add function attributes to this function.
public:
   Attribute getParamAttribute(unsigned ArgNo, Attribute::AttrKind Kind) const;
 
   /// removes noundef and other attributes that imply undefined behavior if a
   /// `undef` or `poison` value is passed from the list of attributes.
   void removeParamUndefImplyingAttrs(unsigned ArgNo);
 
   /// Return the stack alignment for the function.
   MaybeAlign getFnStackAlign() const {
-    return AttributeSets.getFnStackAlignment();
+    return getAttributes().getFnStackAlignment();
   }
 
   /// Returns true if the function has ssp, sspstrong, or sspreq fn attrs.
   bool hasStackProtectorFnAttr() const;
 
   /// adds the dereferenceable attribute to the list of attributes for
   /// the given arg.
   void addDereferenceableParamAttr(unsigned ArgNo, uint64_t Bytes);
 
   /// adds the dereferenceable_or_null attribute to the list of
   /// attributes for the given arg.
   void addDereferenceableOrNullParamAttr(unsigned ArgNo, uint64_t Bytes);
 
   MaybeAlign getParamAlign(unsigned ArgNo) const {
-    return AttributeSets.getParamAlignment(ArgNo);
+    return getAttributes().getParamAlignment(ArgNo);
   }
 
   MaybeAlign getParamStackAlign(unsigned ArgNo) const {
-    return AttributeSets.getParamStackAlignment(ArgNo);
+    return getAttributes().getParamStackAlignment(ArgNo);
   }
 
   /// Extract the byval type for a parameter.
   Type *getParamByValType(unsigned ArgNo) const {
-    return AttributeSets.getParamByValType(ArgNo);
+    return getAttributes().getParamByValType(ArgNo);
   }
 
   /// Extract the sret type for a parameter.
   Type *getParamStructRetType(unsigned ArgNo) const {
-    return AttributeSets.getParamStructRetType(ArgNo);
+    return getAttributes().getParamStructRetType(ArgNo);
   }
 
   /// Extract the inalloca type for a parameter.
   Type *getParamInAllocaType(unsigned ArgNo) const {
-    return AttributeSets.getParamInAllocaType(ArgNo);
+    return getAttributes().getParamInAllocaType(ArgNo);
   }
 
   /// Extract the byref type for a parameter.
   Type *getParamByRefType(unsigned ArgNo) const {
-    return AttributeSets.getParamByRefType(ArgNo);
+    return getAttributes().getParamByRefType(ArgNo);
   }
 
   /// Extract the preallocated type for a parameter.
   Type *getParamPreallocatedType(unsigned ArgNo) const {
-    return AttributeSets.getParamPreallocatedType(ArgNo);
+    return getAttributes().getParamPreallocatedType(ArgNo);
   }
 
   /// Extract the number of dereferenceable bytes for a parameter.
   /// @param ArgNo Index of an argument, with 0 being the first function arg.
   uint64_t getParamDereferenceableBytes(unsigned ArgNo) const {
-    return AttributeSets.getParamDereferenceableBytes(ArgNo);
+    return getAttributes().getParamDereferenceableBytes(ArgNo);
   }
 
   /// Extract the number of dereferenceable_or_null bytes for a
   /// parameter.
   /// @param ArgNo AttributeList ArgNo, referring to an argument.
   uint64_t getParamDereferenceableOrNullBytes(unsigned ArgNo) const {
-    return AttributeSets.getParamDereferenceableOrNullBytes(ArgNo);
+    return getAttributes().getParamDereferenceableOrNullBytes(ArgNo);
   }
 
   /// Determine if the function is presplit coroutine.
   bool isPresplitCoroutine() const {
     return hasFnAttribute(Attribute::PresplitCoroutine);
   }
   void setPresplitCoroutine() { addFnAttr(Attribute::PresplitCoroutine); }
   void setSplittedCoroutine() { removeFnAttr(Attribute::PresplitCoroutine); }
public:
   void setMustProgress() { addFnAttr(Attribute::MustProgress); }
 
   /// Determine if the function will return.
   bool willReturn() const { return hasFnAttribute(Attribute::WillReturn); }
   void setWillReturn() { addFnAttr(Attribute::WillReturn); }
 
   /// Get what kind of unwind table entry to generate for this function.
   UWTableKind getUWTableKind() const {
-    return AttributeSets.getUWTableKind();
+    return getAttributes().getUWTableKind();
   }
 
   /// True if the ABI mandates (or the user requested) that this
   /// function be in a unwind table.
   bool hasUWTable() const {
     return getUWTableKind() != UWTableKind::None;
   }
   void setUWTableKind(UWTableKind K) {
     addFnAttr(Attribute::getWithUWTableKind(getContext(), K));
   }
   /// True if this function needs an unwind table.
   bool needsUnwindTableEntry() const {
     return hasUWTable() || !doesNotThrow() || hasPersonalityFn();
   }
 
   /// Determine if the function returns a structure through first
   /// or second pointer argument.
   bool hasStructRetAttr() const {
-    return AttributeSets.hasParamAttr(0, Attribute::StructRet) ||
-           AttributeSets.hasParamAttr(1, Attribute::StructRet);
+    return getAttributes().hasParamAttr(0, Attribute::StructRet) ||
+           getAttributes().hasParamAttr(1, Attribute::StructRet);
   }
 
   /// Determine if the parameter or return value is marked with NoAlias
   /// attribute.
   bool returnDoesNotAlias() const {
-    return AttributeSets.hasRetAttr(Attribute::NoAlias);
+    return getAttributes().hasRetAttr(Attribute::NoAlias);
   }
   void setReturnDoesNotAlias() { addRetAttr(Attribute::NoAlias); }
 
   /// Do not optimize this function (-O0).
   bool hasOptNone() const { return hasFnAttribute(Attribute::OptimizeNone); }
 
   /// Optimize this function for minimum size (-Oz).
   bool hasMinSize() const { return hasFnAttribute(Attribute::MinSize); }

llvm/lib/IR/Function.cpp

 bool Argument::hasAttribute(Attribute::AttrKind Kind) const {
   return getParent()->hasParamAttribute(getArgNo(), Kind);
 }
 
 Attribute Argument::getAttribute(Attribute::AttrKind Kind) const {
   return getParent()->getParamAttribute(getArgNo(), Kind);
 }
 
+void FnAttributeList::setAttributes(AttributeList NewAttrs) {
+  Attrs = NewAttrs;
+}
+
 //===----------------------------------------------------------------------===//
 // Helper Methods in Function
 //===----------------------------------------------------------------------===//
 
 LLVMContext &Function::getContext() const {
   return getType()->getContext();
 }
 
if (getNumOperands()) {
     setValueSubclassData(getSubclassDataFromValue() & ~0xe);
   }
 
   // Metadata is stored in a side-table.
   clearMetadata();
 }
 
 void Function::addAttributeAtIndex(unsigned i, Attribute Attr) {
-  AttributeSets = AttributeSets.addAttributeAtIndex(getContext(), i, Attr);
+  setAttributes(getAttributes().addAttributeAtIndex(getContext(), i, Attr));
 }
 
 void Function::addFnAttr(Attribute::AttrKind Kind) {
-  AttributeSets = AttributeSets.addFnAttribute(getContext(), Kind);
+  setAttributes(getAttributes().addFnAttribute(getContext(), Kind));
 }
 
 void Function::addFnAttr(StringRef Kind, StringRef Val) {
-  AttributeSets = AttributeSets.addFnAttribute(getContext(), Kind, Val);
+  setAttributes(getAttributes().addFnAttribute(getContext(), Kind, Val));
 }
 
 void Function::addFnAttr(Attribute Attr) {
-  AttributeSets = AttributeSets.addFnAttribute(getContext(), Attr);
+  setAttributes(getAttributes().addFnAttribute(getContext(), Attr));
 }
 
 void Function::addFnAttrs(const AttrBuilder &Attrs) {
-  AttributeSets = AttributeSets.addFnAttributes(getContext(), Attrs);
+  setAttributes(getAttributes().addFnAttributes(getContext(), Attrs));
 }
 
 void Function::addRetAttr(Attribute::AttrKind Kind) {
-  AttributeSets = AttributeSets.addRetAttribute(getContext(), Kind);
+  setAttributes(getAttributes().addRetAttribute(getContext(), Kind));
 }
 
 void Function::addRetAttr(Attribute Attr) {
-  AttributeSets = AttributeSets.addRetAttribute(getContext(), Attr);
+  setAttributes(getAttributes().addRetAttribute(getContext(), Attr));
 }
 
 void Function::addRetAttrs(const AttrBuilder &Attrs) {
-  AttributeSets = AttributeSets.addRetAttributes(getContext(), Attrs);
+  setAttributes(getAttributes().addRetAttributes(getContext(), Attrs));
 }
 
 void Function::addParamAttr(unsigned ArgNo, Attribute::AttrKind Kind) {
-  AttributeSets = AttributeSets.addParamAttribute(getContext(), ArgNo, Kind);
+  setAttributes(getAttributes().addParamAttribute(getContext(), ArgNo, Kind));
 }
 
 void Function::addParamAttr(unsigned ArgNo, Attribute Attr) {
-  AttributeSets = AttributeSets.addParamAttribute(getContext(), ArgNo, Attr);
+  setAttributes(getAttributes().addParamAttribute(getContext(), ArgNo, Attr));
 }
 
 void Function::addParamAttrs(unsigned ArgNo, const AttrBuilder &Attrs) {
-  AttributeSets = AttributeSets.addParamAttributes(getContext(), ArgNo, Attrs);
+  setAttributes(getAttributes().addParamAttributes(getContext(), ArgNo, Attrs));
 }
 
 void Function::removeAttributeAtIndex(unsigned i, Attribute::AttrKind Kind) {
-  AttributeSets = AttributeSets.removeAttributeAtIndex(getContext(), i, Kind);
+  setAttributes(getAttributes().removeAttributeAtIndex(getContext(), i, Kind));
 }
 
 void Function::removeAttributeAtIndex(unsigned i, StringRef Kind) {
-  AttributeSets = AttributeSets.removeAttributeAtIndex(getContext(), i, Kind);
+  setAttributes(getAttributes().removeAttributeAtIndex(getContext(), i, Kind));
 }
 
 void Function::removeFnAttr(Attribute::AttrKind Kind) {
-  AttributeSets = AttributeSets.removeFnAttribute(getContext(), Kind);
+  setAttributes(getAttributes().removeFnAttribute(getContext(), Kind));
 }
 
 void Function::removeFnAttr(StringRef Kind) {
-  AttributeSets = AttributeSets.removeFnAttribute(getContext(), Kind);
+  setAttributes(getAttributes().removeFnAttribute(getContext(), Kind));
 }
 
 void Function::removeFnAttrs(const AttributeMask &AM) {
-  AttributeSets = AttributeSets.removeFnAttributes(getContext(), AM);
+  setAttributes(getAttributes().removeFnAttributes(getContext(), AM));
 }
 
 void Function::removeRetAttr(Attribute::AttrKind Kind) {
-  AttributeSets = AttributeSets.removeRetAttribute(getContext(), Kind);
+  setAttributes(getAttributes().removeRetAttribute(getContext(), Kind));
 }
 
 void Function::removeRetAttr(StringRef Kind) {
-  AttributeSets = AttributeSets.removeRetAttribute(getContext(), Kind);
+  setAttributes(getAttributes().removeRetAttribute(getContext(), Kind));
 }
 
 void Function::removeRetAttrs(const AttributeMask &Attrs) {
-  AttributeSets = AttributeSets.removeRetAttributes(getContext(), Attrs);
+  setAttributes(getAttributes().removeRetAttributes(getContext(), Attrs));
 }
 
 void Function::removeParamAttr(unsigned ArgNo, Attribute::AttrKind Kind) {
-  AttributeSets = AttributeSets.removeParamAttribute(getContext(), ArgNo, Kind);
+  setAttributes(
+      getAttributes().removeParamAttribute(getContext(), ArgNo, Kind));
 }
 
 void Function::removeParamAttr(unsigned ArgNo, StringRef Kind) {
-  AttributeSets = AttributeSets.removeParamAttribute(getContext(), ArgNo, Kind);
+  setAttributes(
+      getAttributes().removeParamAttribute(getContext(), ArgNo, Kind));
 }
 
 void Function::removeParamAttrs(unsigned ArgNo, const AttributeMask &Attrs) {
-  AttributeSets =
-      AttributeSets.removeParamAttributes(getContext(), ArgNo, Attrs);
+  setAttributes(
+      getAttributes().removeParamAttributes(getContext(), ArgNo, Attrs));
 }
 
 void Function::addDereferenceableParamAttr(unsigned ArgNo, uint64_t Bytes) {
-  AttributeSets =
-      AttributeSets.addDereferenceableParamAttr(getContext(), ArgNo, Bytes);
+  setAttributes(
+      getAttributes().addDereferenceableParamAttr(getContext(), ArgNo, Bytes));
 }
 
 bool Function::hasFnAttribute(Attribute::AttrKind Kind) const {
-  return AttributeSets.hasFnAttr(Kind);
+  return getAttributes().hasFnAttr(Kind);
 }
 
 bool Function::hasFnAttribute(StringRef Kind) const {
-  return AttributeSets.hasFnAttr(Kind);
+  return getAttributes().hasFnAttr(Kind);
 }
 
 bool Function::hasRetAttribute(Attribute::AttrKind Kind) const {
-  return AttributeSets.hasRetAttr(Kind);
+  return getAttributes().hasRetAttr(Kind);
 }
 
 bool Function::hasParamAttribute(unsigned ArgNo,
                                  Attribute::AttrKind Kind) const {
-  return AttributeSets.hasParamAttr(ArgNo, Kind);
+  return getAttributes().hasParamAttr(ArgNo, Kind);
 }
 
 Attribute Function::getAttributeAtIndex(unsigned i,
                                         Attribute::AttrKind Kind) const {
-  return AttributeSets.getAttributeAtIndex(i, Kind);
+  return getAttributes().getAttributeAtIndex(i, Kind);
 }
 
 Attribute Function::getAttributeAtIndex(unsigned i, StringRef Kind) const {
-  return AttributeSets.getAttributeAtIndex(i, Kind);
+  return getAttributes().getAttributeAtIndex(i, Kind);
 }
 
 Attribute Function::getFnAttribute(Attribute::AttrKind Kind) const {
-  return AttributeSets.getFnAttr(Kind);
+  return getAttributes().getFnAttr(Kind);
 }
 
 Attribute Function::getFnAttribute(StringRef Kind) const {
-  return AttributeSets.getFnAttr(Kind);
+  return getAttributes().getFnAttr(Kind);
 }
 
 uint64_t Function::getFnAttributeAsParsedInteger(StringRef Name,
                                                  uint64_t Default) const {
   Attribute A = getFnAttribute(Name);
   uint64_t Result = Default;
   if (A.isStringAttribute()) {
     StringRef Str = A.getValueAsString();
     if (Str.getAsInteger(0, Result))
       getContext().emitError("cannot parse integer attribute " + Name);
   }
 
   return Result;
 }
 
 /// gets the specified attribute from the list of attributes.
 Attribute Function::getParamAttribute(unsigned ArgNo,
                                       Attribute::AttrKind Kind) const {
-  return AttributeSets.getParamAttr(ArgNo, Kind);
+  return getAttributes().getParamAttr(ArgNo, Kind);
 }
 
 void Function::addDereferenceableOrNullParamAttr(unsigned ArgNo,
                                                  uint64_t Bytes) {
-  AttributeSets = AttributeSets.addDereferenceableOrNullParamAttr(getContext(),
-                                                                  ArgNo, Bytes);
+  setAttributes(getAttributes().addDereferenceableOrNullParamAttr(
+      getContext(), ArgNo, Bytes));
 }
 
 DenormalMode Function::getDenormalMode(const fltSemantics &FPType) const {
   if (&FPType == &APFloat::IEEEsingle()) {
     Attribute Attr = getFnAttribute("denormal-fp-math-f32");
     StringRef Val = Attr.getValueAsString();
     if (!Val.empty())
       return parseDenormalFPAttribute(Val);