Extend function attributes bitset size from 64 to 96.

[eugenis]

Summary

We are going to add a function attribute number 64.

[eugenis]

I've moved the bitset into the padding of NumAttrSets so hopefully this will not waste any memory.

[jdoerfert]

Thanks for working on this. Making sure we can extend attributes is a bit of a hassle now that we reached the magic number ;)

This looks fine to me but I'm not to involved in these parts. Let's see if someone else want to approve this.

---

We are going to add a function attribute number 64.

Out of curiosity, which one do you mean here?

[eugenis]

It's sanitize_memtag in the child revision.
Actually, we were fine without this until your change on Jule 10 pushed our pending attribute over the limit :)

[vitalybuka]

llvm/lib/IR/AttributeImpl.h

250 ↗

(On Diff #209572)

std::bitset?

[jdoerfert]

In D64663#1583507, @eugenis wrote:

It's sanitize_memtag in the child revision.
Actually, we were fine without this until your change on Jule 10 pushed our pending attribute over the limit :)

It seems it's my fault...

(sorry for derailing this, always wanted to try these memes they added)

llvm/lib/IR/AttributeImpl.h

250 ↗

(On Diff #209572)

We also have a llvm::SmallBitVector. If we keep a fixed size encoding we should add a (static) assert so it'll break in a informative manner once the new limit is reached.

[eugenis]

std::bitset is word-aligned at least in libc++ implementation, so it would waste 4 bytes here.
SmallBitVector has only 64 bits of inline storage.

[pcc]

I would prefer to keep this the way it is with an array so that it is obvious what the size/alignment are.

It would be good to have a static assertion of some sort, though. It looks like Attribute::EndAttrKinds could be used for this purpose.

[eugenis]

In D64663#1583672, @pcc wrote:

I would prefer to keep this the way it is with an array so that it is obvious what the size/alignment are.

It would be good to have a static assertion of some sort, though. It looks like Attribute::EndAttrKinds could be used for this purpose.

We already have that in AttributeListImpl::AttributeListImpl constructor.

[pcc]

Ah, I missed that.

LGTM

llvm/lib/IR/AttributeImpl.h

250 ↗

(On Diff #209572)

Nit: maybe do uint8_t AvailableFunctionAttrs[12] = {}; to avoid needing to put the initializer in the ctor.

[eugenis]

oops, found another place to update

[eugenis]

Tested by adding a bunch of fake attributes at the beginning to push the interesting ones out of 0..64 range, and also with ASan.

llvm/trunk/lib/IR/AttributeImpl.h

 /// \class
 /// This class represents a group of attributes that apply to one
 /// element: function, return type, or parameter.
 class AttributeSetNode final
     : public FoldingSetNode,
       private TrailingObjects<AttributeSetNode, Attribute> {
   friend TrailingObjects;
 
-  /// Bitset with a bit for each available attribute Attribute::AttrKind.
-  uint64_t AvailableAttrs;
   unsigned NumAttrs; ///< Number of attributes in this node.
+  /// Bitset with a bit for each available attribute Attribute::AttrKind.
+  uint8_t AvailableAttrs[12] = {};
 
   AttributeSetNode(ArrayRef<Attribute> Attrs);
 
 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, const AttrBuilder &B);
 
   static AttributeSetNode *get(LLVMContext &C, ArrayRef<Attribute> Attrs);
 
   /// Return the number of attributes this AttributeList contains.
   unsigned getNumAttributes() const { return NumAttrs; }
 
   bool hasAttribute(Attribute::AttrKind Kind) const {
-    return AvailableAttrs & ((uint64_t)1) << Kind;
+    return AvailableAttrs[Kind / 8] & ((uint64_t)1) << (Kind % 8);
   }
   bool hasAttribute(StringRef Kind) const;
   bool hasAttributes() const { return NumAttrs != 0; }
 
   Attribute getAttribute(Attribute::AttrKind Kind) const;
   Attribute getAttribute(StringRef Kind) const;
 
   unsigned getAlignment() const;

 /// return type, and parameters.
 class AttributeListImpl final
     : public FoldingSetNode,
       private TrailingObjects<AttributeListImpl, AttributeSet> {
   friend class AttributeList;
   friend TrailingObjects;
 
 private:
-  /// Bitset with a bit for each available attribute Attribute::AttrKind.
-  uint64_t AvailableFunctionAttrs;
   LLVMContext &Context;
   unsigned NumAttrSets; ///< Number of entries in this set.
+  /// Bitset with a bit for each available attribute Attribute::AttrKind.
+  uint8_t AvailableFunctionAttrs[12] = {};
 
   // Helper fn for TrailingObjects class.
   size_t numTrailingObjects(OverloadToken<AttributeSet>) { return NumAttrSets; }
 
 public:
   AttributeListImpl(LLVMContext &C, ArrayRef<AttributeSet> Sets);
 
   // AttributesSetImpt is uniqued, these should not be available.
   AttributeListImpl(const AttributeListImpl &) = delete;
   AttributeListImpl &operator=(const AttributeListImpl &) = delete;
 
   void operator delete(void *p) { ::operator delete(p); }
 
   /// Get the context that created this AttributeListImpl.
   LLVMContext &getContext() { return Context; }
 
   /// Return true if the AttributeSet or the FunctionIndex has an
   /// enum attribute of the given kind.
   bool hasFnAttribute(Attribute::AttrKind Kind) const {
-    return AvailableFunctionAttrs & ((uint64_t)1) << Kind;
+    return AvailableFunctionAttrs[Kind / 8] & ((uint64_t)1) << (Kind % 8);
   }
 
   using iterator = const AttributeSet *;
 
   iterator begin() const { return getTrailingObjects<AttributeSet>(); }
   iterator end() const { return begin() + NumAttrSets; }
 
   void Profile(FoldingSetNodeID &ID) const;
   static void Profile(FoldingSetNodeID &ID, ArrayRef<AttributeSet> Nodes);
 
   void dump() const;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_IR_ATTRIBUTEIMPL_H

llvm/trunk/lib/IR/Attributes.cpp

 }
 #endif
 
 //===----------------------------------------------------------------------===//
 // AttributeSetNode Definition
 //===----------------------------------------------------------------------===//
 
 AttributeSetNode::AttributeSetNode(ArrayRef<Attribute> Attrs)
-    : AvailableAttrs(0), NumAttrs(Attrs.size()) {
+    : NumAttrs(Attrs.size()) {
   // There's memory after the node where we can store the entries in.
   llvm::copy(Attrs, getTrailingObjects<Attribute>());
 
+  static_assert(Attribute::EndAttrKinds <=
+                    sizeof(AvailableAttrs) * CHAR_BIT,
+                "Too many attributes");
+
   for (const auto I : *this) {
     if (!I.isStringAttribute()) {
-      AvailableAttrs |= ((uint64_t)1) << I.getKindAsEnum();
+      Attribute::AttrKind Kind = I.getKindAsEnum();
+      AvailableAttrs[Kind / 8] |= 1ULL << (Kind % 8);
     }
   }
 }
 
 AttributeSetNode *AttributeSetNode::get(LLVMContext &C,
                                         ArrayRef<Attribute> Attrs) {
   if (Attrs.empty())
     return nullptr;

 /// unsigned wrapping in constexpr functions, so write out the conditional. LLVM
 /// folds it to add anyway.
 static constexpr unsigned attrIdxToArrayIdx(unsigned Index) {
   return Index == AttributeList::FunctionIndex ? 0 : Index + 1;
 }
 
 AttributeListImpl::AttributeListImpl(LLVMContext &C,
                                      ArrayRef<AttributeSet> Sets)
-    : AvailableFunctionAttrs(0), Context(C), NumAttrSets(Sets.size()) {
+    : Context(C), NumAttrSets(Sets.size()) {
   assert(!Sets.empty() && "pointless AttributeListImpl");
 
   // There's memory after the node where we can store the entries in.
   llvm::copy(Sets, getTrailingObjects<AttributeSet>());
 
   // Initialize AvailableFunctionAttrs summary bitset.
   static_assert(Attribute::EndAttrKinds <=
                     sizeof(AvailableFunctionAttrs) * CHAR_BIT,
                 "Too many attributes");
   static_assert(attrIdxToArrayIdx(AttributeList::FunctionIndex) == 0U,
                 "function should be stored in slot 0");
   for (const auto I : Sets[0]) {
-    if (!I.isStringAttribute())
-      AvailableFunctionAttrs |= 1ULL << I.getKindAsEnum();
+    if (!I.isStringAttribute()) {
+      Attribute::AttrKind Kind = I.getKindAsEnum();
+      AvailableFunctionAttrs[Kind / 8] |= 1ULL << (Kind % 8);
+    }
   }
 }
 
 void AttributeListImpl::Profile(FoldingSetNodeID &ID) const {
   Profile(ID, makeArrayRef(begin(), end()));
 }
 
 void AttributeListImpl::Profile(FoldingSetNodeID &ID,