[PATCH] D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D63845#1562006 <https://reviews.llvm.org/D63845#1562006>, @wsmoses wrote:

> Just to preface these comments -- this is work in progress and not intended to go in as is / without discussion (but happily is indeed inviting discussion).


Fantastic!

> In D63845#1561983 <https://reviews.llvm.org/D63845#1561983>, @lebedev.ri wrote:
> 
>> In D63845#1561793 <https://reviews.llvm.org/D63845#1561793>, @jdoerfert wrote:
>>
>> > In D63845#1560605 <https://reviews.llvm.org/D63845#1560605>, @aaron.ballman wrote:
>> >
>> > > In D63845#1559995 <https://reviews.llvm.org/D63845#1559995>, @lebedev.ri wrote:
>> > >
>> > > > What's the target use-case here? What can't be solved with normal attributes?
>> > >
>> >
>> >
>> > With "normal" you mean something like `__attribute__((noescape))`? We don't have them for all attributes, I doubt we want to but I might be wrong.
>>
>>
>> That is precisely the question.
>>  What is the motivation for exposing such LLVM-specific low-level unstable implementation detail?
> 
> 
> There's a couple of potential use cases for this -- most of which are more for LLVM developers than end users.

That's why I am hesitant to provide it as an end-user feature. I have not seen a compelling case for why a generic set of user-facing attributes is a good design as opposed to thinking about individual attributes LLVM exposes and carefully thinking how we want to allow users access to it (if at all), which is the status quo. Once we expose this sort of feature to users, they *will* use it even if it isn't meant for them, and you can't unring a bell. What stability guarantees do these attributes have?  How are you planning to handle semantic checking of the attributes (so that attributes which only appertain to some constructs are properly diagnosed when written on the wrong construct)? How do you plan to handle attributes that require arguments (as well as diagnosing improper argument types, etc)? How do you diagnose mutually exclusive backend attributes? These sort of questions are ones that all get answered when exposing individual attributes as needed, but they're problems that need to be solved in a generic way for these three attributes. Also, you're not exposing any statement attributes through this -- does the LLVM backend not have statement-level attributes? What about type attributes?

> The primary driver behind this is a research/GSOC project to try propagating information learned in LLVM analysis/optimization passes back into augmented headers.

To what end? If we inferred this information successfully, do we gain something by explicitly putting it into a header? One way of exposing these attributes that would make me marginally more comfortable would be to always diagnose use of these attributes as being experimental, with no warning flag group to allow the warning to be disabled. However, if you plan to write these attributes into headers and then distribute those headers for production use, that approach won't work.

> I've also personally run into situations where I've been either trying create new attributes for LLVM or generating test cases for optimizations that require specific LLVM attributes that otherwise don't exist with Clang's existing frontend.

I don't think this is a motivating use case for our users, though.

>>>>> How is versioning expected to be handled? New attribute vs old clang, and vice versa.
>>> 
>>> Unknown attributes are not necessarily a problem, they should compile fine ignoring the attribute, or am I wrong?
>> 
>> I don't know, that's why i'm asking.

The issue I see is that it will be temptingly easy to ignore the attributes silently rather than ignoring the attributes by telling the user they're being ignored and why. The other issue I see is that we (to my knowledge) have no stability guarantees with LLVM attributes that are exposed this way, and this raises the implementation burden for backend attributes.

> Off the top of my head, there's a couple of ways we could imagine dealing with versioning:
> 
> - We could have the equivalent of the LLVM/Clang version included as part of the attribute (e.g. something like `__attribute__((llvm7_arg(a, "nocapture")))`)

This will not scale well, but we could add it as an argument to the attribute itself.

> - We could let nonexistent attributes simply slide into extraneous string attributes. In its current form it will look for an enum attribute if one exists and if not create a string attribute. I don't believe that extraneous string attributes would cause any harm -- but correct me if I'm wrong.

I'm not okay with this -- nonexistent attributes need to be diagnosed rather than silently ignored.

> - We could separate string attributes from enum attributes and raise errors if an enum attribute doesn't exist. This doesn't cover semantic changes, but provides more information than creating it as an extra string attribute (at cost of possible compiler error for the user).

This is a good first step, but it doesn't solve a lot of the issues I raised above regarding other diagnosable attribute scenarios.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63845/new/

https://reviews.llvm.org/D63845