[RFC] We are running out of slots in the Attribute::AttrKind enum

I would like to add a target-dependent attribute to the LLVM IR, and the guidance in http://llvm.org/docs/HowToUseAttributes.html says that target-dependent attributes should not occupy a slot in the Attribute::AttrKind enum, but I have yet to find an attribute that is represented in the IR that does not also have a slot in the AttrKind enum.

We are limited to 63 slots in the AttrKind enum because it is represented in the bitcode with a 64-bit bitmask. There is only one free slot left and I don’t want to use it for a target-dependent attribute. I had anticipated that only target-independent attributes would have analogous ATTR_KIND identifiers in the AtttrKind enum, yet the ARM-specific CMSE_NS_CALL and CMSE_NS_ENTRY occupy slots in the AttrKind enum.

Can someone point me to an example of a target-dependent attribute represented in the LLVM IR that does not occupy a slot in the AttrKind enum?

Thanks,

Todd Snider

We should also consider an extension. Maybe using the last bit to
indicate a lookup in a second 64-bit mask, or if the memory footprint is
not a problem, even a bitvector implementation.

There is certainly interest in adding more attributes (I can provide
references if needed), making them faster would certainly not hurt.

Cheers,
  Johannes

Is anyone in the community already looking at extending the implementation of AvailableFunctionAttrs beyond a uint64_t bitmask?

Would a bit vector replacement be an amenable alternative for bitcode encodings of attributes?

The llvm.org guidelines suggest that there should be some delineation between target-independent and target-dependent attributes, but I can't find evidence of this in the way that attributes are represented in the LLVM IR. Am I missing something? Should we be moving attributes in a direction similar to the way that intrinsics are supported in the source base?

~ Todd

Why aren’t string attributes appropriate for your use case? Why do you need the attribute to be a recognized enum?

I cannot find any references to CMSE_NS_CALL anywhere in trunk LLVM. The only result google gave for me for it was:
https://www.iar.com/globalassets/about-us/events/ew2019/ew2019-working-effectively-with-trustzone-devices.pdf

I looked at the latest code for writing bitcode, and I don’t think it uses a bitmask to store enum attributes anymore. See this code:
https://github.com/llvm-git-prototype/llvm/blob/a845b0985672ee66c1cc8e070ca5d5ac6e89c0c9/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp#L728

So, I don’t think there are any compatibility issues with adding more attributes.

In your next message, you mention AvailableFunctionAttrs, and I think all that needs to be done is to rewrite it to use std::bitset, as is done in AttrBuilder.

Reid, thanks for the response. I think I now have a path forward for adding target-dependent attributes as string attributes instead of enum attributes.

For informational purposes, after digging deeper into how the bitcode reader and writer work for enum attributes, I conclude:

· Attributes are encoded in the bitcode file as groups

· Each enum attribute within a group is identified by its encoding value which matches the value is was given in the AttrKind enum specification (i.e. its ATTR_KIND_xxx value)

· BitcodeWriter handling of enum attributes:

o The bitcode writer has an EnumAttributeImpl version of the attribute kind which you can get from Attr.getKindAsEnum(); I believe this is the index into the Attribute specification table that gets generated from Attributes.td when llvm/clang is built

o Calling getAttrKindEncoding(x) where x is Attr.getKindAsEnum() will then present the encoding value that goes into the bitcode file to identify the attribute kind

o There is not a limitation on the size of the AttrKind enum. However, there is a static assert (~line 820+ - AttributeListImpl()) that enforces that the number of slots occupied in the AttrKind enum is less than (sizeof(AvailableFunctionAttrs) * CHAR_BITS). Since AvailableFunctionAttrs currently has type uint64_t, this static assert needs to be updated or removed (it is currently incorrectly limiting the enum to 64 slots – including the Attribute::none slot).

· BitcodeReader handling of enum attributes:

o The reader no longer uses the getRawAttributeMask() or addRawAttributeValue() functions. I had incorrectly assumed that they were still in play.

o The reader uses getAttrFromCode() to map the encoded value of the attribute to the Attribute specification index (i.e. ATTR_KIND_xxx → Attribute::xxx)

The reason I became concerned about running out of slots in the AttrKind enum is because my work group has a couple of target-independent attributes that we will be looking to upstream in the near future. When I attempted to merge changes from the upstream repo into our local source base (that contains an implementation of those attributes), the static assert mentioned above prevented a successful build.

I propose to update or remove the above static assert so that it correctly reflects the current state of the bitcode reader/writer with respect to enum attributes.

Does that make sense?

~ Todd Snider