[RFC] Overhauling Attributes

Overhauling Attributes

Problem

I love it in principle. As you get closer to finalizing the syntax, etc. I might have some more detailed comments, but nothing really substantive.

I love it in principle. As you get closer to finalizing the syntax, etc. I might have some more detailed comments, but nothing really substantive.

Yeah. I have a notion of what I'd like the syntax to look like, but that's not what this proposal is for. This is basically a "heads up!" that I'm about to change the Attributes class.

Minor API comment as that seems more immediate:

An example syntax could be:

// Building an Attribute

Attributes A;
A.addAlignAttr(4)
.addNoUnwindAttr()
.addStackProtectorAttr()
.addUnwindTableAttr()
.addReadNoneAttr();

Personally, I would prefer to make the Attributes class be immutable, and the building happen in a helper. One possible interface would end up looking like:

Attributes Base = ...;

Attributes A = Attributes::Builder(Base).addNoUnwind()
                                        .addStackProtector()
                                        .addAlign(4);

I would like that too. Hmm...This has potential. Though I'll need to see how it plays out in practice. A lot of uses look like they assume a mutable object...

-bw

If you make syntax changes please send patches to Pygments so that our
Sphinx docs remain pretty. Sphinx handles failures to lex in an
extremely non-graceful way; basically, it's policy is that if the code
it is asked to format as a particular language doesn't match it's
lexer, then it just gives up and leaves the entire block of code
unhighlighted :frowning:

Even so much as using a keyword it doesn't recognize will cause it to
bail out immediately and leave the `.. code-block::` completely
unhighlighted. The relevant part of Pygments is
<https://bitbucket.org/birkenfeld/pygments-main/src/e97df81b39c7/pygments/lexers/asm.py#cl-193&gt;;
it's pretty self-explanatory.

...aside... When I was attempting to Sphinxify LangRef, I actually ran
into a couple such instances where new keywords had been introduced. I
never finished that attempt since I eventually realized that LangRef
is 8000 lines of HTML: some kind of automated conversion is going to
be necessary (even with fancy vim macros and stuff my ETA was still
unreasonable).

--Sean Silva

Yep, that’s the goal. Eventually “Attributes” will be a pointer to a uniqued (and thus, immutable) object managed by llvmcontext. Having a mutable builder is really important, because you don’t want to be re-uniquing for each additional attribute crammed on in a chain.

-Chris

Hi Bill,

Problem

LTO needs a way to pass options through to different parts of the compiler. In
particular, we need to pass code generation options to the back-end. The way we
want to do this is via the LLVM Attributes class. In order to do that, we need
to overhaul the Attributes class.

The Attributes class right now isn't very extensible. After considering several
different options, we decided it was best to extend the Attributes class to
support *all* code generation options, even target-specific ones. It already
supports some of them (like SSP), but it isn't very extensible because it is a
fixed size bitfield. The size restriction also makes supporting options with
values very difficult. As the number of options grow, we will soon have run out
of space entirely.

Target-Specific Attributes in IR

I propose that the target-specific attributes would be organized into groups.
Those groups would then be referenced by the functions directly. Here is an
example of what that could look like:

attrgroup #1 = { "long-calls", "cpu=cortex-a8", "thumb" }

define void @func() noinline ssp attrgroup(#1) {
   ret void
}

does this mean you would like attributes to form a tree structure? This sounds
a lot like metadata, only metadata that can't be thrown away - do you think it
feasible to have this and metadata share code, eg each be derived from some
common parent class?

Allowing values for attributes would also be great for implementing something
Chris suggested in one of his notes: getting rid of the "zext" attribute, which
means that some (eg) i16 parameter is magically zext'd to an unspecified larger
type like i32, and instead having the larger type i32 be the parameter type
with a "zexted from i16" attribute on it. This could be done as "zexted=i16"
with your system I guess.

Ciao, Duncan.

Hi Bill,

Problem

LTO needs a way to pass options through to different parts of the compiler. In
particular, we need to pass code generation options to the back-end. The way we
want to do this is via the LLVM Attributes class. In order to do that, we need
to overhaul the Attributes class.

The Attributes class right now isn't very extensible. After considering several
different options, we decided it was best to extend the Attributes class to
support *all* code generation options, even target-specific ones. It already
supports some of them (like SSP), but it isn't very extensible because it is a
fixed size bitfield. The size restriction also makes supporting options with
values very difficult. As the number of options grow, we will soon have run out
of space entirely.

Target-Specific Attributes in IR

I propose that the target-specific attributes would be organized into groups.
Those groups would then be referenced by the functions directly. Here is an
example of what that could look like:

attrgroup #1 = { "long-calls", "cpu=cortex-a8", "thumb" }

define void @func() noinline ssp attrgroup(#1) {
  ret void
}

does this mean you would like attributes to form a tree structure? This sounds
a lot like metadata, only metadata that can't be thrown away - do you think it
feasible to have this and metadata share code, eg each be derived from some
common parent class?

Not really. I want this to be separate from metadata. I don't expect that attribute groups will reference each other. And I want to keep the attributes that are stored as light weight as possible. However, I haven't come up with the complete proposal just yet.

Allowing values for attributes would also be great for implementing something
Chris suggested in one of his notes: getting rid of the "zext" attribute, which
means that some (eg) i16 parameter is magically zext'd to an unspecified larger
type like i32, and instead having the larger type i32 be the parameter type
with a "zexted from i16" attribute on it. This could be done as "zexted=i16"
with your system I guess.

Yeah, something like that. I'll look at Chris's note and see if I can incorporate it into the final design. :slight_smile:

-bw

attrgroup #1 = { "long-calls", "cpu=cortex-a8", "thumb" }

define void @func() noinline ssp attrgroup(#1) {
  ret void
}

I like the general idea. Just one clarification: In the above example,
are the attributes taken from a list specified in the language ref
(like current attributes) or can they be arbitrary "strings" (like
metadata)?

Cheers,
Rafael

Hi Rafael,

Sorry, I forgot to respond to this. They can be arbitrary strings that are known only to the specific back-end. It may be beneficial to define them inside of the LangRef document though.

-bw

Hi Rafael,

Sorry, I forgot to respond to this. They can be arbitrary strings that are known only to the specific back-end. It may be beneficial to define them inside of the LangRef document though.

But then, what are the semantics that the middle end should use? Can a
non thumb function be inlined in a thumb one for example? If we merge
two functions, how should we combine the attributes? Or should this
not be a valid optimization at all?

-bw

Cheers,
Rafael

Hi Bill,

Hi Rafael,

Sorry, I forgot to respond to this. They can be arbitrary strings that are known only to the specific back-end. It may be beneficial to define them inside of the LangRef document though.

this sounds so much like metadata... What was the reason for not enhancing
metadata to cover this use case? I'm sure you explained but I've completely
forgotten...

Ciao, Duncan.

this sounds so much like metadata... What was the reason for not enhancing
metadata to cover this use case? I'm sure you explained but I've completely
forgotten...

I was assuming the difference was that attributes cannot be removed in
general. But then the only options I can see are
* Document the semantics of all of them.
* Find some other generic way by which the middle end can handle an
unknown attribute.

Ciao, Duncan.

Cheers,
Rafael

All good questions. The initial implementation will be a very conservative "if the attributes don't match, then we won't inline them." Yes, that's horribly bad in general, but shouldn't be too much of a problem in non-LTO cases. This can be relaxed later on. For target-specific attributes, we will have to rely upon the back-end to tell us if it's okay to inline them.

-bw

Hi Duncan,

There are a couple of reasons why I don't think it's a good idea to use metadata (or more specifically, the module-level flags). Firstly, everything would have to be specified as a strings: "noinline", "ssp", etc., because a metadata object can hold only a Value type. Secondly, I don't want to have a "loop" in the attributes:

  !1 = metadata !{ !"noinline", !2 }
  !2 = metadata !{ !1, !"ssp" }

This makes uniquifying the attributes that much harder.

We also want to be able to intelligently merge the attribute groups. These two groups are identical:

  #1 = attributes { noredzone noinline sspreq "mcpu"="cortex-a8" }
  #2 = attributes { "mcpu"="cortex-a8" sspreq noinline noredzone }

Which brings up the '<kind>=<value>' syntax. The ability to have (possibly multiple) values assigned to an attribute kind isn't something easily modeled in metadata, as far as I can tell.

These (and maybe a few more, it's late) are some of the reasons why I feel a new first-class IR construct is needed. It's possible to modify metadata to handle these. But I feel that it's much more cumbersome to do that.

-bw

Hi Bill,

This looks cool.

I'm definitely in need of this.

What is the recommendation for us to use for now until this is checked in.

I have attributes mips16 and nomips16 to add for Mips.

These would be function attributes.

Reed

I would like to add something to atttributes.h, attributes.cpp in the interim until your full scheme is available.

A new attribute called "target" would be added to AttrKind.

And target can take a list of strings.

target("foo", "goo")

For example.

I would add a component targetAttrs to AttrBuilder

Will this meet with resistance if I try and put this back?

Reed

Looks like new attribute work is moving along quickly

Maybe I should just wait.

???

Oops! Sorry...This got skipped by me.

I'm not sure what the recommended approach should be here. I don't like you modifying the IR to add the 'target' keyword. Would this be an acceptable hack? Create a global variable "@llvm.mips16" or whatever. That global would list the functions which have that attribute. Those which aren't in the list would have the attribute "nomips16".

Alternatively, you could create a module flag which takes a list of functions that have that attribute:

!0 = metadata !{ i32 1, metadata !"mips16", !{ i32 ()* @foo, i32 ()* @bar } }

!llvm.module.flags = !{ !0 }

-bw