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

[wsmoses]

Summary

This (work in progress) diff adds three new attributes to clang (attribute((LLVMFN(string))), attribute((LLVMARG(paramidx, string))), attribute((LLVMRET(paramidx, string))) ) for passing attributes down to LLVM.

This is useful for allowing users, tools, or test cases to specify LLVM attributes that may not have a representation in clang.

[wsmoses]

Actually upload full patch so far (and not just the last commit).

[jdoerfert]

Please add full context to the patches (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)
Also, some general comments inlined.

clang/include/clang/Basic/Attr.td

1652

You will eventually need to add documentation for all three.

clang/lib/CodeGen/CodeGenFunction.cpp

761

Variable names: VarName
Comments and clang-format, please.
assert needs a text
Can we merge the common argument and return attribute code?
Why don't we need to parse the AttrKind for FnAttr?

[lebedev.ri]

What's the target use-case here? What can't be solved with normal attributes?
I wonder if this should go to cfe+llvm -dev lists first, it's kinda intrusive.
I also wonder if all these should cause a clang diagnostic, at least under -Wall.
How is versioning expected to be handled? New attribute vs old clang, and vice versa.

[aaron.ballman]

In D63845#1559995, @lebedev.ri wrote:

What's the target use-case here? What can't be solved with normal attributes?
I wonder if this should go to cfe+llvm -dev lists first, it's kinda intrusive.
I also wonder if all these should cause a clang diagnostic, at least under -Wall.
How is versioning expected to be handled? New attribute vs old clang, and vice versa.

Also, we already have the annotate attribute for passing information through to LLVM. Why do we need four different ways to do this?

clang/include/clang/Basic/Attr.td

1638

This is not a GCC attribute, so this needs a different spelling. Also, the all-caps names are a bit obnoxious, so we may want to pick a different name.

Do you envision these being attributes users will write in production code? What kind of guarantees do they have about the stability of the attribute interfaces that LLVM provides? For instance, are we promising that LLVM attributes will remain backwards compatible (e.g., won't be removed or renamed)?

[jdoerfert]

In D63845#1560605, @aaron.ballman wrote:

In 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.

I wonder if this should go to cfe+llvm -dev lists first, it's kinda intrusive.

This is not a review to get this in. Before anything happens an RFC will be issued and discussed. We'll use this to experiment and other people can do so as well.

I also wonder if all these should cause a clang diagnostic, at least under -Wall.

What do you mean?

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?

Also, we already have the annotate attribute for passing information through to LLVM. Why do we need four different ways to do this?

I didn't know that one, e.g., it is not in https://clang.llvm.org/docs/AttributeReference.html. I glanced it in clang and it did not seem to do the same thing, thus allowing to generate LLVM-IR attributes at the different positions. If it is, we should use it instead.

[lebedev.ri]

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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?

I wonder if this should go to cfe+llvm -dev lists first, it's kinda intrusive.

This is not a review to get this in. Before anything happens an RFC will be issued and discussed. We'll use this to experiment and other people can do so as well.

I also wonder if all these should cause a clang diagnostic, at least under -Wall.

What do you mean?

These are very much a generally-incompatible, very fragile, extensions.
They will not work on any other compiler, and will likely not work
on a different version of clang. It is not okay not to diagnose such cases.

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.

Also, we already have the annotate attribute for passing information through to LLVM. Why do we need four different ways to do this?

I didn't know that one, e.g., it is not in https://clang.llvm.org/docs/AttributeReference.html. I glanced it in clang and it did not seem to do the same thing, thus allowing to generate LLVM-IR attributes at the different positions. If it is, we should use it instead.

[wsmoses]

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).

In D63845#1561983, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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.

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

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.

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.

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"))))
• 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.
• 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).

[aaron.ballman]

In 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, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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.

[jdoerfert]

In D63845#1561983, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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?

I would disagree to the unstable part. Keeping LLVM-IR backward compatibility is always a priority and changing the meaning of an existing LLVM-IR attribute will break various things already.
Why would you think it is unstable? Also, we basically do expose the low-level parts one by one through "normal" attributes as soon as someone has enough motivation to write all the necessary boilerplate (see more below). Why not merge all the logic/code-gen into a single uniform framework that deals with LLVM-IR attributes?

I also wonder if all these should cause a clang diagnostic, at least under -Wall.

What do you mean?

These are very much a generally-incompatible, very fragile, extensions.
They will not work on any other compiler, and will likely not work
on a different version of clang. It is not okay not to diagnose such cases.

It totally depends on your definition of "will not work". They might not be recognized and therefore be ignored, correct. They should not be misinterpreted as (1) the name should make them unambiguous and (2) LLVM-IR needs to be backward compatible anyway. Take different version of clang and __attribute__((noescape)) for example, or take gcc. All the problems apply but it is even far less obvious that it is not a language feature and will not be portable (across compilers). (Given that various commercial compilers are build on top of LLVM/Clang there is an argument for forward compatibility.) I agree on the diagnose issue, we should, as we do, warn for unrecognized attributes. But again, both clang and gcc do that by default.

As a side node, I actually want to provide user with a "macro-portability-layer" that hides the spelling and even allows to remove the attributes for incompatible compilers. Though, one step at a time.

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.

(See above.)

In D63845#1564289, @aaron.ballman wrote:

In D63845#1561983, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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?

These are a lot of very good questions we have to answer at some point. I hope we can do so in a constructive and goal oriented fashion instead of using them as a excuse to block advancing the status quo. Let me try to give partial answers right away in an effort to show there is a way forward (in addition to the scientific project we will use them for):

Use-cases:

There are three, albeit similar, use cases that come to mind:
1) A performance-aware user interprets analysis remarks (or debug messages) and provides the information that was missing for the optimization they had in mind to fire. This is not an artificial use case, we have that with performance-aware people here (in the labs) and we see emails on the list that ask for help to enable transformations.
2) We have tooling support that helps less involved/advanced users to determine performance improving opportunities (see https://link.springer.com/chapter/10.1007/978-3-030-20656-7_13) and we need to provide them with a way to actually make use of them.
3) We want to distribute a library in non-lto mode but without introducing optimization barriers at each call to it. This is the project @wsmoses is working right now (see https://www.llvm.org/OpenProjects.html#header-generation).

Stability: I argued above already that LLVM-IR attributes are, or better have to be, backward compatible in practice. You would already break systems if they were not.

Semantic checking, usage, arguments: For the prototype we have the IR verifier to do this job. Before we (try to) upstream any of this, we need to expose the checking capabilities and use them in the sema. This has to happen. Though, I do not see it as a major obstacle. First, we can whitelist attributes that are allowed in the new scheme (the big switch). Second, (generic) IR attributes are fairly simple, if they have arguments it is a single integer. Lastly, the number of locations IR attributes are placed is fairly limited, the interesting ones are: (a) function/call site attributes, (b) argument/parameter attributes, (c) return value/call site value 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.

Let's try it out for science sake, without trying to commit it.
Also, as mention above already, all the problems mentioned so far, portability/compatibility, stability, etc. exist already for the hand picked attributes we have.
What do you think of a "macro-portability-layer"?

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.

Agreed. This can be a bonus but is not sufficient for integration.

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.

I mentioned warnings, whitelisting, and existing stability guarantees already. Does that lower your wariness?

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.

I do not believe we need to but I do not have the strongest feeling here. Especially with a "macro-layer" this would be easy to do anyway.

• 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.

Agreed.

• 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.

We have warnings for unknown "C-level" attributes, e.g., warning: unknown attribute 'noescapedsa' ignored [-Wunknown-attributes], and we have to issue these warnings for unknown LLVM-IR attributes as well.

[aaron.ballman]

In D63845#1566987, @jdoerfert wrote:

In D63845#1561983, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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?

I would disagree to the unstable part. Keeping LLVM-IR backward compatibility is always a priority and changing the meaning of an existing LLVM-IR attribute will break various things already.
Why would you think it is unstable?

Because, to date, we've never made blanket any stability guarantees about LLVM attributes from within the context of Clang. Instead, LLVM attributes are introduced into Clang on a case by case basis when they've been determined to be stable enough to warrant exposing. This extra layer also gives us more flexibility in translating frontend attributes into backend attributes.

Also, we basically do expose the low-level parts one by one through "normal" attributes as soon as someone has enough motivation to write all the necessary boilerplate (see more below). Why not merge all the logic/code-gen into a single uniform framework that deals with LLVM-IR attributes?

Because that increases the chance of bad user experiences by lowering the bar that ensures newly added attributes behave well for user code. That boilerplate exists for a reason and forces a developer to make decisions (like what number and kind of arguments are accepted, what spellings make sense for the attribute, what subjects the attribute appertains to, etc).

I also wonder if all these should cause a clang diagnostic, at least under -Wall.

What do you mean?

These are very much a generally-incompatible, very fragile, extensions.
They will not work on any other compiler, and will likely not work
on a different version of clang. It is not okay not to diagnose such cases.

It totally depends on your definition of "will not work". They might not be recognized and therefore be ignored, correct. They should not be misinterpreted as (1) the name should make them unambiguous and (2) LLVM-IR needs to be backward compatible anyway. Take different version of clang and __attribute__((noescape)) for example, or take gcc. All the problems apply but it is even far less obvious that it is not a language feature and will not be portable (across compilers). (Given that various commercial compilers are build on top of LLVM/Clang there is an argument for forward compatibility.) I agree on the diagnose issue, we should, as we do, warn for unrecognized attributes. But again, both clang and gcc do that by default.

As a side node, I actually want to provide user with a "macro-portability-layer" that hides the spelling and even allows to remove the attributes for incompatible compilers. Though, one step at a time.

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.

(See above.)

In D63845#1564289, @aaron.ballman wrote:

In D63845#1561983, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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?

These are a lot of very good questions we have to answer at some point. I hope we can do so in a constructive and goal oriented fashion instead of using them as a excuse to block advancing the status quo.

My intentions are not to block advancement of the status quo, but when given a patch with a concrete implementation that lacks much of the polish required for a production-quality feature, it's reasonable to push back strongly. This isn't a small feature, so the bar for inclusion is higher.

Let me try to give partial answers right away in an effort to show there is a way forward (in addition to the scientific project we will use them for):

Use-cases:

There are three, albeit similar, use cases that come to mind:
1) A performance-aware user interprets analysis remarks (or debug messages) and provides the information that was missing for the optimization they had in mind to fire. This is not an artificial use case, we have that with performance-aware people here (in the labs) and we see emails on the list that ask for help to enable transformations.
2) We have tooling support that helps less involved/advanced users to determine performance improving opportunities (see https://link.springer.com/chapter/10.1007/978-3-030-20656-7_13) and we need to provide them with a way to actually make use of them.
3) We want to distribute a library in non-lto mode but without introducing optimization barriers at each call to it. This is the project @wsmoses is working right now (see https://www.llvm.org/OpenProjects.html#header-generation).

Stability: I argued above already that LLVM-IR attributes are, or better have to be, backward compatible in practice. You would already break systems if they were not.

Semantic checking, usage, arguments: For the prototype we have the IR verifier to do this job. Before we (try to) upstream any of this, we need to expose the checking capabilities and use them in the sema. This has to happen. Though, I do not see it as a major obstacle. First, we can whitelist attributes that are allowed in the new scheme (the big switch). Second, (generic) IR attributes are fairly simple, if they have arguments it is a single integer. Lastly, the number of locations IR attributes are placed is fairly limited, the interesting ones are: (a) function/call site attributes, (b) argument/parameter attributes, (c) return value/call site value attributes.

Having done a lot of work to generalize our attribute tablegen to automate semantic checking like this, I see this as more of an obstacle than you do. I suspect you will find that only about 80% of attributes fit the easy cases you envision because there will be attributes that need special requirements (late parsing, template instantiation behaviors, target-specific attributes, etc) and that will be Very Hard to get right. User experience is the biggest concern I have with this feature.

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.

Let's try it out for science sake, without trying to commit it.

Trying it out in a private branch is a great idea. :-)

Also, as mention above already, all the problems mentioned so far, portability/compatibility, stability, etc. exist already for the hand picked attributes we have.
What do you think of a "macro-portability-layer"?

I think it's an interesting idea, but the important point is that we (to date) *hand pick* attributes from LLVM and bring them into Clang. This is an automation of that process, and that automation is what I'm pushing back against without more information about how we plan to expose these attributes responsibly. That means considering things like "should this attribute be exposed in <language>?", "can this conflict with other attributes", "should this attribute only apply to static global variables, or are thread local variables also of interest", appropriate documentation, etc.

Basically: LLVM attributes are designed with backend requirements in mind, and backend requirements are not generally as user-facing as frontend attributes. What safeguards can be put in place to ensure that these frontend requirements are forced to be considered before exposing a new backend attribute?

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.

Agreed. This can be a bonus but is not sufficient for integration.

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.

I mentioned warnings, whitelisting, and existing stability guarantees already. Does that lower your wariness?

Yes, but you only mentioned that they needed to be handled -- I'd like to know how with more concrete details. My big worry is that we trivially expose backend attributes that are arcane, undocumented (for frontend use), or provide a bad user experience. As a more concrete example of what I'm afraid of -- we have an LLVM function attribute named "thunk". How does Clang do the semantic checking for that attribute -- it needs to diagnose the attribute when it's ignored, and so if the function never makes a tail call, what diagnostics does the user get? How do you rewrite that documentation so that it makes sense to a frontend user?

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.

I do not believe we need to but I do not have the strongest feeling here. Especially with a "macro-layer" this would be easy to do anyway.

• 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.

Agreed.

• 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.

We have warnings for unknown "C-level" attributes, e.g., warning: unknown attribute 'noescapedsa' ignored [-Wunknown-attributes], and we have to issue these warnings for unknown LLVM-IR attributes as well.

Unknown attributes are not the only reason an attribute is ignored, though. I am more worried about *known* attributes that are ignored because of code issues, like my example with "thunk" above. I think it should be easy to automate generic checking for things like number of arguments and ignoring target-specific attributes on unsupported targets. I think it will be a little bit harder to ensure attributes appertain to the proper subject and have the correct argument types, but it shouldn't be too bad. I think the other semantic checks that lead to attributes being ignored will be the intractable problem.

I'm wondering if we can leverage tablegen to solve these concerns. <speculation>What if we used tablegen (from the backend) to generate the skeleton concrete frontend attribute implementation in some sort of automated fashion? So, instead of adding an attribute to a keeplist and using a generic attribute syntax (as in this patch), we exposed the backend attributes like we would any other frontend attribute (using a tablegen member to determine which backend attributes are exposed)? I'm thinking of something like a Clang build target that reaches back into LLVM for its attribute tablegen file and generates an LLVMBackendAttrs.td file for the frontend to consume (that gets included from Attr.td and AttrDocs.td). It could additionally make a SemaDeclLLVMBackendAttrs.inc file to be included into SemaDeclAttr.cpp for declaration attribute processing. This won't cover all of the cases I'm worried about (such as more complex semantic checking), but it gives an easy path to exposing backend attributes to the frontend in a way that's consistent with our other attributes.</speculation> Something where we generated "real" attributes for these would also cut down on the chances of us having code like if (const auto *A = SomeDecl->getAttr<LLVMFNAttr>()) { if (A->getBackendAttrKind() == "whatever") { ... } } in the frontend.

[jdoerfert]

I think we should postpone a detailed discussion until we are ready to send an RFC on this. Nevertheless, I inlined some responses below.

In D63845#1570639, @aaron.ballman wrote:

In D63845#1566987, @jdoerfert wrote:

In D63845#1561983, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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?

I would disagree to the unstable part. Keeping LLVM-IR backward compatibility is always a priority and changing the meaning of an existing LLVM-IR attribute will break various things already.
Why would you think it is unstable?

Because, to date, we've never made blanket any stability guarantees about LLVM attributes from within the context of Clang. Instead, LLVM attributes are introduced into Clang on a case by case basis when they've been determined to be stable enough to warrant exposing. This extra layer also gives us more flexibility in translating frontend attributes into backend attributes.

You still assume LLVM attributes are not stable while I tried to argue they are, they have to be already for backward compatibility, at least target agnostic ones.

Your example below was thunk and you mentioned other problems that can arise but I think we should step back for a moment and talk about the use case here because that should limit the potential problems. For me the use case is as follows: The user, or a tool, wants to encode additional information in the source to aide optimization. The basically only way to do this in IR is to add attributes to one of three locations, (1) function level, (2) return value level, or (3) argument level. For all three we have examples in Clang so we know how do interact with templates, and other potential hurdles. Now it is not the goal to allow all attributes as some of them encode not information but implementation details, e.g., thunk. I'm basically interested in attributes that can be safely derived and dropped in he IR today.

Also, we basically do expose the low-level parts one by one through "normal" attributes as soon as someone has enough motivation to write all the necessary boilerplate (see more below). Why not merge all the logic/code-gen into a single uniform framework that deals with LLVM-IR attributes?

Because that increases the chance of bad user experiences by lowering the bar that ensures newly added attributes behave well for user code. That boilerplate exists for a reason and forces a developer to make decisions (like what number and kind of arguments are accepted, what spellings make sense for the attribute, what subjects the attribute appertains to, etc).

We can still make all these decisions, this patch almost makes all of them explicit. We can whitelist arguments (through the switch that exists), we can rename them between the FE and IR if we want to (though I would prefer not to), and we do restrict the positions already to the three interesting ones.

In D63845#1564289, @aaron.ballman wrote:

In D63845#1561983, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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?

These are a lot of very good questions we have to answer at some point. I hope we can do so in a constructive and goal oriented fashion instead of using them as a excuse to block advancing the status quo.

My intentions are not to block advancement of the status quo, but when given a patch with a concrete implementation that lacks much of the polish required for a production-quality feature, it's reasonable to push back strongly. This isn't a small feature, so the bar for inclusion is higher.

Maybe my wording was off but I found it "problematic" that there was harsh critique on a first draft clearly marked as WIP not looking for actual code review (no llvm-commits subscriber). Maybe this would have needed more background. The patch is part of a GSoC project to explore options and it is clearly not ready for prime time yet. I asked for it to be put here so I can take a look and comment in a familiar environment. Also other peoples feedback is obviously welcomed. The tablegen idea and other things mentioned are very interesting.

Let me try to give partial answers right away in an effort to show there is a way forward (in addition to the scientific project we will use them for):

Use-cases:

There are three, albeit similar, use cases that come to mind:
1) A performance-aware user interprets analysis remarks (or debug messages) and provides the information that was missing for the optimization they had in mind to fire. This is not an artificial use case, we have that with performance-aware people here (in the labs) and we see emails on the list that ask for help to enable transformations.
2) We have tooling support that helps less involved/advanced users to determine performance improving opportunities (see https://link.springer.com/chapter/10.1007/978-3-030-20656-7_13) and we need to provide them with a way to actually make use of them.
3) We want to distribute a library in non-lto mode but without introducing optimization barriers at each call to it. This is the project @wsmoses is working right now (see https://www.llvm.org/OpenProjects.html#header-generation).

Stability: I argued above already that LLVM-IR attributes are, or better have to be, backward compatible in practice. You would already break systems if they were not.

Semantic checking, usage, arguments: For the prototype we have the IR verifier to do this job. Before we (try to) upstream any of this, we need to expose the checking capabilities and use them in the sema. This has to happen. Though, I do not see it as a major obstacle. First, we can whitelist attributes that are allowed in the new scheme (the big switch). Second, (generic) IR attributes are fairly simple, if they have arguments it is a single integer. Lastly, the number of locations IR attributes are placed is fairly limited, the interesting ones are: (a) function/call site attributes, (b) argument/parameter attributes, (c) return value/call site value attributes.

Having done a lot of work to generalize our attribute tablegen to automate semantic checking like this, I see this as more of an obstacle than you do. I suspect you will find that only about 80% of attributes fit the easy cases you envision because there will be attributes that need special requirements (late parsing, template instantiation behaviors, target-specific attributes, etc) and that will be Very Hard to get right. User experience is the biggest concern I have with this feature.

I really think that >95% of all LLVM-IR optimization attributes are easy. There are certainly other attributes but most of them come with all the problems you described and therefore do not fit a general scheme.

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.

Let's try it out for science sake, without trying to commit it.

Trying it out in a private branch is a great idea. :-)

We will before an RFC and a patch for review is produced.

Also, as mention above already, all the problems mentioned so far, portability/compatibility, stability, etc. exist already for the hand picked attributes we have.
What do you think of a "macro-portability-layer"?

I think it's an interesting idea, but the important point is that we (to date) *hand pick* attributes from LLVM and bring them into Clang. This is an automation of that process, and that automation is what I'm pushing back against without more information about how we plan to expose these attributes responsibly. That means considering things like "should this attribute be exposed in <language>?", "can this conflict with other attributes", "should this attribute only apply to static global variables, or are thread local variables also of interest", appropriate documentation, etc.

Basically: LLVM attributes are designed with backend requirements in mind, and backend requirements are not generally as user-facing as frontend attributes. What safeguards can be put in place to ensure that these frontend requirements are forced to be considered before exposing a new backend attribute?

That is not generally true. There are LLVM "backend attributes", or better "implementation attributes", and LLVM "information attributes". Only the the latter is targeted here and that is where we are constantly adding more of. Information attributes are target agnostic and describe properties of the IR (values). The safeguard is the whitelist mentioned before.

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.

I mentioned warnings, whitelisting, and existing stability guarantees already. Does that lower your wariness?

Yes, but you only mentioned that they needed to be handled -- I'd like to know how with more concrete details. My big worry is that we trivially expose backend attributes that are arcane, undocumented (for frontend use), or provide a bad user experience. As a more concrete example of what I'm afraid of -- we have an LLVM function attribute named "thunk". How does Clang do the semantic checking for that attribute -- it needs to diagnose the attribute when it's ignored, and so if the function never makes a tail call, what diagnostics does the user get? How do you rewrite that documentation so that it makes sense to a frontend user?

Why would be whitelist an arcane, undocumented attribute? I think you paint the picture to negative here. (I commented on thunk and similar attributes above.)

• 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.

We have warnings for unknown "C-level" attributes, e.g., warning: unknown attribute 'noescapedsa' ignored [-Wunknown-attributes], and we have to issue these warnings for unknown LLVM-IR attributes as well.

Unknown attributes are not the only reason an attribute is ignored, though. I am more worried about *known* attributes that are ignored because of code issues, like my example with "thunk" above. I think it should be easy to automate generic checking for things like number of arguments and ignoring target-specific attributes on unsupported targets. I think it will be a little bit harder to ensure attributes appertain to the proper subject and have the correct argument types, but it shouldn't be too bad. I think the other semantic checks that lead to attributes being ignored will be the intractable problem.

I want "information attributes" describing "properties", such as: dereferenceable(_globally), align, nonull, noreturn, returned, read/write-only, readnone, noescape, inaccessiblememonly_XXX, nounwind, ...
Some of these have already a counterpart in Clang, others do not. These do come with semantic "implications" but I don't think the semantic checks we need are particularly hard. We need to make sure they are used at a suitable position (out of the three), and the type of the underlying value is acceptable, e.g., a pointer type for align. If you want to check if they actually hold, or trivially not hold, that is a different question which I would not try to answer right now.

I'm wondering if we can leverage tablegen to solve these concerns. <speculation>What if we used tablegen (from the backend) to generate the skeleton concrete frontend attribute implementation in some sort of automated fashion? So, instead of adding an attribute to a keeplist and using a generic attribute syntax (as in this patch), we exposed the backend attributes like we would any other frontend attribute (using a tablegen member to determine which backend attributes are exposed)? I'm thinking of something like a Clang build target that reaches back into LLVM for its attribute tablegen file and generates an LLVMBackendAttrs.td file for the frontend to consume (that gets included from Attr.td and AttrDocs.td). It could additionally make a SemaDeclLLVMBackendAttrs.inc file to be included into SemaDeclAttr.cpp for declaration attribute processing. This won't cover all of the cases I'm worried about (such as more complex semantic checking), but it gives an easy path to exposing backend attributes to the frontend in a way that's consistent with our other attributes.</speculation> Something where we generated "real" attributes for these would also cut down on the chances of us having code like if (const auto *A = SomeDecl->getAttr<LLVMFNAttr>()) { if (A->getBackendAttrKind() == "whatever") { ... } } in the frontend.

I like it. That would be a nice solution to generate the checking and annotation code in a generic manner.

[aaron.ballman]

In D63845#1571855, @jdoerfert wrote:

I think we should postpone a detailed discussion until we are ready to send an RFC on this. Nevertheless, I inlined some responses below.

In D63845#1570639, @aaron.ballman wrote:

In D63845#1566987, @jdoerfert wrote:

In D63845#1561983, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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?

I would disagree to the unstable part. Keeping LLVM-IR backward compatibility is always a priority and changing the meaning of an existing LLVM-IR attribute will break various things already.
Why would you think it is unstable?

Because, to date, we've never made blanket any stability guarantees about LLVM attributes from within the context of Clang. Instead, LLVM attributes are introduced into Clang on a case by case basis when they've been determined to be stable enough to warrant exposing. This extra layer also gives us more flexibility in translating frontend attributes into backend attributes.

You still assume LLVM attributes are not stable while I tried to argue they are, they have to be already for backward compatibility, at least target agnostic ones.

I think *many* LLVM attributes have stability to them, especially once they've had some time to bake. It's the odd ones that don't fit the norm that have me uncomfortable. However, because the proposal isn't a blanket addition of *all* LLVM attributes automatically, I'm far less concerned. I didn't pick up on that bit from the original proposal.

Your example below was thunk and you mentioned other problems that can arise but I think we should step back for a moment and talk about the use case here because that should limit the potential problems. For me the use case is as follows: The user, or a tool, wants to encode additional information in the source to aide optimization. The basically only way to do this in IR is to add attributes to one of three locations, (1) function level, (2) return value level, or (3) argument level. For all three we have examples in Clang so we know how do interact with templates, and other potential hurdles. Now it is not the goal to allow all attributes as some of them encode not information but implementation details, e.g., thunk. I'm basically interested in attributes that can be safely derived and dropped in he IR today.

That seems like a reasonable decision, to me. I picked on thunk only because it was a good example of an attribute that would require additional semantic checks that couldn't be easily automated, but I'm sure there's a subset of LLVM attributes that have easier semantics to deal with. Thank you for the extra clarification.

Also, we basically do expose the low-level parts one by one through "normal" attributes as soon as someone has enough motivation to write all the necessary boilerplate (see more below). Why not merge all the logic/code-gen into a single uniform framework that deals with LLVM-IR attributes?

Because that increases the chance of bad user experiences by lowering the bar that ensures newly added attributes behave well for user code. That boilerplate exists for a reason and forces a developer to make decisions (like what number and kind of arguments are accepted, what spellings make sense for the attribute, what subjects the attribute appertains to, etc).

We can still make all these decisions, this patch almost makes all of them explicit. We can whitelist arguments (through the switch that exists), we can rename them between the FE and IR if we want to (though I would prefer not to), and we do restrict the positions already to the three interesting ones.

Fantastic! I think that if you take the tablegen approach I suggested, we can hopefully (easily) cover all the random bits of automated checking we've added for frontend attribute checking.

In D63845#1564289, @aaron.ballman wrote:

In D63845#1561983, @lebedev.ri wrote:

In D63845#1561793, @jdoerfert wrote:

In D63845#1560605, @aaron.ballman wrote:

In 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?

These are a lot of very good questions we have to answer at some point. I hope we can do so in a constructive and goal oriented fashion instead of using them as a excuse to block advancing the status quo.

My intentions are not to block advancement of the status quo, but when given a patch with a concrete implementation that lacks much of the polish required for a production-quality feature, it's reasonable to push back strongly. This isn't a small feature, so the bar for inclusion is higher.

Maybe my wording was off but I found it "problematic" that there was harsh critique on a first draft clearly marked as WIP not looking for actual code review (no llvm-commits subscriber). Maybe this would have needed more background. The patch is part of a GSoC project to explore options and it is clearly not ready for prime time yet. I asked for it to be put here so I can take a look and comment in a familiar environment. Also other peoples feedback is obviously welcomed. The tablegen idea and other things mentioned are very interesting.

Oh holy wow! I did not notice that this hadn't gone out to llvm-commits or cfe-commits for review -- I thought this was a typical patch, which is why I pushed back as strongly as I did (despite the WIP in the title, even). I'm sorry for not noticing these details! I had the impression that this specific design was more concrete than it probably is.

Let me try to give partial answers right away in an effort to show there is a way forward (in addition to the scientific project we will use them for):

Use-cases:

There are three, albeit similar, use cases that come to mind:
1) A performance-aware user interprets analysis remarks (or debug messages) and provides the information that was missing for the optimization they had in mind to fire. This is not an artificial use case, we have that with performance-aware people here (in the labs) and we see emails on the list that ask for help to enable transformations.
2) We have tooling support that helps less involved/advanced users to determine performance improving opportunities (see https://link.springer.com/chapter/10.1007/978-3-030-20656-7_13) and we need to provide them with a way to actually make use of them.
3) We want to distribute a library in non-lto mode but without introducing optimization barriers at each call to it. This is the project @wsmoses is working right now (see https://www.llvm.org/OpenProjects.html#header-generation).

Stability: I argued above already that LLVM-IR attributes are, or better have to be, backward compatible in practice. You would already break systems if they were not.

Semantic checking, usage, arguments: For the prototype we have the IR verifier to do this job. Before we (try to) upstream any of this, we need to expose the checking capabilities and use them in the sema. This has to happen. Though, I do not see it as a major obstacle. First, we can whitelist attributes that are allowed in the new scheme (the big switch). Second, (generic) IR attributes are fairly simple, if they have arguments it is a single integer. Lastly, the number of locations IR attributes are placed is fairly limited, the interesting ones are: (a) function/call site attributes, (b) argument/parameter attributes, (c) return value/call site value attributes.

Having done a lot of work to generalize our attribute tablegen to automate semantic checking like this, I see this as more of an obstacle than you do. I suspect you will find that only about 80% of attributes fit the easy cases you envision because there will be attributes that need special requirements (late parsing, template instantiation behaviors, target-specific attributes, etc) and that will be Very Hard to get right. User experience is the biggest concern I have with this feature.

I really think that >95% of all LLVM-IR optimization attributes are easy. There are certainly other attributes but most of them come with all the problems you described and therefore do not fit a general scheme.

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.

Let's try it out for science sake, without trying to commit it.

Trying it out in a private branch is a great idea. :-)

We will before an RFC and a patch for review is produced.

Sounds great!

Also, as mention above already, all the problems mentioned so far, portability/compatibility, stability, etc. exist already for the hand picked attributes we have.
What do you think of a "macro-portability-layer"?

I think it's an interesting idea, but the important point is that we (to date) *hand pick* attributes from LLVM and bring them into Clang. This is an automation of that process, and that automation is what I'm pushing back against without more information about how we plan to expose these attributes responsibly. That means considering things like "should this attribute be exposed in <language>?", "can this conflict with other attributes", "should this attribute only apply to static global variables, or are thread local variables also of interest", appropriate documentation, etc.

Basically: LLVM attributes are designed with backend requirements in mind, and backend requirements are not generally as user-facing as frontend attributes. What safeguards can be put in place to ensure that these frontend requirements are forced to be considered before exposing a new backend attribute?

That is not generally true.

I disagree; people who typically review frontend attributes are not the same people who review backend attributes. I have definitely had frontend reviews exposing backend attributes where the author has not thought about all of the messier aspects of the frontend languages the attribute is exposed in.

There are LLVM "backend attributes", or better "implementation attributes", and LLVM "information attributes". Only the the latter is targeted here and that is where we are constantly adding more of. Information attributes are target agnostic and describe properties of the IR (values). The safeguard is the whitelist mentioned before.

Ah, I didn't know you made a distinction between the two in the backend, that's good to know. Target agnostic attributes are easier than ones with target-specific semantics, but the kinds of design considerations I'm worried about are target-agnostic things like "should this apply to ObjC method decls?" and "what happens when this is written on a virtual function -- how do overriders behave?". The keeplist doesn't safeguard those sort of questions as strongly as a full attribute implementation does, but I think we can find a happy medium that isn't too onerous for anyone.

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.

I mentioned warnings, whitelisting, and existing stability guarantees already. Does that lower your wariness?

Yes, but you only mentioned that they needed to be handled -- I'd like to know how with more concrete details. My big worry is that we trivially expose backend attributes that are arcane, undocumented (for frontend use), or provide a bad user experience. As a more concrete example of what I'm afraid of -- we have an LLVM function attribute named "thunk". How does Clang do the semantic checking for that attribute -- it needs to diagnose the attribute when it's ignored, and so if the function never makes a tail call, what diagnostics does the user get? How do you rewrite that documentation so that it makes sense to a frontend user?

Why would be whitelist an arcane, undocumented attribute? I think you paint the picture to negative here. (I commented on thunk and similar attributes above.)

Backend attributes can be arcane by their nature; thunk is a great example of that. We have backend documentation for that attribute, but it is not super useful to a Clang user ("The caller is expected to cast the thunk prototype to match the thunk target prototype." -- what does that *mean* to someone adding this attribute to a C function declaration?). What I meant by undocumented is two-fold: 1) it is important that we have *frontend* docs for these attributes that explain the attribute behavior (it doesn't have to go directly into AttrDocs.td, but should be published to the same online resource as AttrDocs.td is), and 2) the frontend docs should not be a copy/paste of the backend docs (or a simple link to them) unless the backend docs are already sufficiently clear to a frontend user, including things like code examples and how the attribute behaves in whatever languages its exposed in. Additionally, we have no test coverage demonstrating proper and improper use of backend attributes like thunk in the frontend, which doesn't mean the attribute is automatically arcane, but lacking this sort of testing makes the attribute somewhat more mysterious than it needs to be for someone unfamiliar with the backend attribute semantics. FWIW, this is a lesson we learned with some of our attributes in the frontend; they're totally undocumented and have almost zero test coverage so when questions arise about the attributes, it becomes an unpleasant code archaeology expedition.

• 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.

We have warnings for unknown "C-level" attributes, e.g., warning: unknown attribute 'noescapedsa' ignored [-Wunknown-attributes], and we have to issue these warnings for unknown LLVM-IR attributes as well.

Unknown attributes are not the only reason an attribute is ignored, though. I am more worried about *known* attributes that are ignored because of code issues, like my example with "thunk" above. I think it should be easy to automate generic checking for things like number of arguments and ignoring target-specific attributes on unsupported targets. I think it will be a little bit harder to ensure attributes appertain to the proper subject and have the correct argument types, but it shouldn't be too bad. I think the other semantic checks that lead to attributes being ignored will be the intractable problem.

I want "information attributes" describing "properties", such as: dereferenceable(_globally), align, nonull, noreturn, returned, read/write-only, readnone, noescape, inaccessiblememonly_XXX, nounwind, ...
Some of these have already a counterpart in Clang, others do not. These do come with semantic "implications" but I don't think the semantic checks we need are particularly hard. We need to make sure they are used at a suitable position (out of the three), and the type of the underlying value is acceptable, e.g., a pointer type for align. If you want to check if they actually hold, or trivially not hold, that is a different question which I would not try to answer right now.

Thank you for this list, as it helps me to understand the scope of what you're after. I do agree that many of the semantic checks for these should be easy, but those are not the checks I worry about. Let me pick on noreturn for a moment (I know this is already exposed in Clang, but it's an easy example of what I'm thinking about). The semantic checks I worry about there are: what happens if I write noreturn on a virtual function (do all overrides have to be noreturn, or can they vary? if they vary, what happens when calling through a base or derived pointer?)? Can I write noreturn on a function that has a non-void return type? Should we diagnose a return statement in such a function? What about a throw statement? What happens when I bind such a function to a function pointer -- does the function pointer have the same semantics? (And so on.)

I think it's important that we be asking, answering, documenting, and testing these sort of things on a per-attribute basis as part of this feature. I don't think this is something we will be able to automate from the backend, so I'm hoping we can find a way to have just enough process in place that these questions get asked but without so much process that it becomes an annoying slog for anyone involved.

I'm wondering if we can leverage tablegen to solve these concerns. <speculation>What if we used tablegen (from the backend) to generate the skeleton concrete frontend attribute implementation in some sort of automated fashion? So, instead of adding an attribute to a keeplist and using a generic attribute syntax (as in this patch), we exposed the backend attributes like we would any other frontend attribute (using a tablegen member to determine which backend attributes are exposed)? I'm thinking of something like a Clang build target that reaches back into LLVM for its attribute tablegen file and generates an LLVMBackendAttrs.td file for the frontend to consume (that gets included from Attr.td and AttrDocs.td). It could additionally make a SemaDeclLLVMBackendAttrs.inc file to be included into SemaDeclAttr.cpp for declaration attribute processing. This won't cover all of the cases I'm worried about (such as more complex semantic checking), but it gives an easy path to exposing backend attributes to the frontend in a way that's consistent with our other attributes.</speculation> Something where we generated "real" attributes for these would also cut down on the chances of us having code like if (const auto *A = SomeDecl->getAttr<LLVMFNAttr>()) { if (A->getBackendAttrKind() == "whatever") { ... } } in the frontend.

I like it. That would be a nice solution to generate the checking and annotation code in a generic manner.

Other questions I have that may be worth considering early: 1) what spellings do we want to support for these? e.g., should they only be exposed via [[]] or is there a reason we should also support __attribute__(()) spellings? 2) should these go under the clang attribute namespace? What sort of name collisions do we have to worry about if it does? Or should they be exposed under an llvm (or some other) namespace?

My suggested answers are: 1) only expose as [[]] spellings to avoid accidentally conflicting with GNU attributes that share the same name with different semantics, 2) I don't know if we already have naming collisions between LLVM attributes and clang attributes, but if we do (or are worried about them in the future), I think having an llvm namespace for these attributes would make the most sense. Also, I can imagine other vendors like GCC wanting to support attributes in the clang namespace (like we do with gnu attributes), but I don't imagine anyone wanting to support these LLVM attributes the same way, so the logical division may make sense.

I'm guessing there are other questions that will come up as we get into it.

Btw, thank you for working on this -- while I have been critical of the patch due to my own misunderstandings, I do see the utility in this general idea.

[jdoerfert]

ping, could we come up with a design for this functionality?

llvm/lib/IR/Attributes.cpp

522

can't we create this via tablegen?

clang/include/clang/Basic/Attr.td

 // A list of identifiers matching parameters or ParamIdx indices.
 class VariadicParamOrParamIdxArgument<string name> : Argument<name, 1>;
 
+// Like VariadicParamOrParamIdxArgument but for a single function parameter index.
+class ParamOrParamIdxArgument<string name, bit opt = 0> : Argument<name, opt>;
+
 // Like VariadicParamIdxArgument but for a single function parameter index.
 class ParamIdxArgument<string name, bit opt = 0> : Argument<name, opt>;
 

   let Documentation = [AllocAlignDocs];
 }
 
+def LLVMFN : InheritableAttr {
+  let Spellings = [GCC<"LLVMFN">];
+  let Args = [StringArgument<"AttrName">];
+  let Documentation = [Undocumented];
+}
+
+def LLVMARG : InheritableAttr {
+  let Spellings = [GCC<"LLVMARG">];
+  let Args = [ParamOrParamIdxArgument<"ParamIndex">, StringArgument<"AttrName">];
+  let Documentation = [Undocumented];
+}
+
+def LLVMRET : InheritableAttr {
+  let Spellings = [GCC<"LLVMRET">];
+  let Args = [StringArgument<"AttrName">];
+  let Documentation = [Undocumented];
+}
+
 def NoReturn : InheritableAttr {
   let Spellings = [GCC<"noreturn">, Declspec<"noreturn">];
   // FIXME: Does GCC allow this on the function instead?

clang/lib/CodeGen/CodeGenFunction.cpp

       SanOpts.Mask &= ~SanitizerKind::CFIUnrelatedCast;
   }
 
+  if (D) {
+    for(auto a: D->attrs()) {
+        if (auto atr = dyn_cast<LLVMFNAttr>(a)) {
+            Fn->addFnAttr(atr->getAttrName());
+        }
+        if (auto atr = dyn_cast<LLVMARGAttr>(a)) {
+            std::pair<StringRef, StringRef> attributeandvalue = atr->getAttrName().split('=');
+            unsigned index = llvm::AttributeList::FirstArgIndex + atr->getParamIndex(); //.getLLVMIndex();
+            llvm::Attribute::AttrKind attrkind = llvm::Attribute::parseAttrKind(attributeandvalue.first);
+            if (attrkind != llvm::Attribute::None) {
+                assert(attributeandvalue.second.size() == 0);
+                Fn->addParamAttr(index, llvm::Attribute::get(Fn->getContext(), attrkind));
+            }
+            else
+                Fn->addParamAttr(index, llvm::Attribute::get(Fn->getContext(), attributeandvalue.first, attributeandvalue.second));
+        }
+        if (auto atr = dyn_cast<LLVMRETAttr>(a)) {
+            std::pair<StringRef, StringRef> attributeandvalue = atr->getAttrName().split('=');
+            unsigned index = llvm::AttributeList::ReturnIndex;
+            llvm::Attribute::AttrKind attrkind = llvm::Attribute::parseAttrKind(attributeandvalue.first);
+            if (attrkind != llvm::Attribute::None) {
+                assert(attributeandvalue.second.size() == 0);
+                Fn->addParamAttr(index, llvm::Attribute::get(Fn->getContext(), attrkind));
+            }
+            else
+                Fn->addParamAttr(index, llvm::Attribute::get(Fn->getContext(), attributeandvalue.first, attributeandvalue.second));
+        }
+    }
+  }
+
   // Apply xray attributes to the function (as a string, for now)
   if (D) {
     if (const auto *XRayAttr = D->getAttr<XRayInstrumentAttr>()) {

clang/lib/Sema/SemaDeclAttr.cpp

     D->addAttr(NewAttr);
 }
 
-/// Handle __attribute__((callback(CalleeIdx, PayloadIdx0, ...))) attributes.
-static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
-  // The index that identifies the callback callee is mandatory.
-  if (AL.getNumArgs() == 0) {
-    S.Diag(AL.getLoc(), diag::err_callback_attribute_no_callee)
-        << AL.getRange();
-    return;
-  }
-
-  bool HasImplicitThisParam = isInstanceMethod(D);
-  int32_t NumArgs = getFunctionOrMethodNumParams(D);
-
-  FunctionDecl *FD = D->getAsFunction();
-  assert(FD && "Expected a function declaration!");
+static int32_t getParamFromNameOrIndex(Sema &S, FunctionDecl *FD, const ParsedAttr &AL, unsigned I) {
+  bool HasImplicitThisParam = isInstanceMethod(FD);
+  int32_t NumArgs = getFunctionOrMethodNumParams(FD);
 
   llvm::StringMap<int> NameIdxMapping;
   NameIdxMapping["__"] = -1;

 
   auto UnknownName = NameIdxMapping.end();
 
-  SmallVector<int, 8> EncodingIndices;
-  for (unsigned I = 0, E = AL.getNumArgs(); I < E; ++I) {
     SourceRange SR;
     int32_t ArgIdx;
 

       if (It == UnknownName) {
         S.Diag(AL.getLoc(), diag::err_callback_attribute_argument_unknown)
             << IdLoc->Ident << IdLoc->Loc;
-        return;
+        return -1;
       }
 
       SR = SourceRange(IdLoc->Loc);

                                false)) {
         S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds)
             << AL << (I + 1) << IdxExpr->getSourceRange();
-        return;
+        return -1;
       }
 
       // Check oob, excluding the special values, 0 and -1.
       if (ArgIdx < -1 || ArgIdx > NumArgs) {
         S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds)
             << AL << (I + 1) << IdxExpr->getSourceRange();
-        return;
+        return -1;
       }
 
       SR = IdxExpr->getSourceRange();

     if (ArgIdx == 0 && !HasImplicitThisParam) {
       S.Diag(AL.getLoc(), diag::err_callback_implicit_this_not_available)
           << (I + 1) << SR;
-      return;
+      return -1;
     }
 
     // Adjust for the case we do not have an implicit "this" parameter. In this

     if (!HasImplicitThisParam && ArgIdx > 0)
       ArgIdx -= 1;
 
+    return ArgIdx;
+}
+
+/// Handle __attribute__((callback(CalleeIdx, PayloadIdx0, ...))) attributes.
+static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // The index that identifies the callback callee is mandatory.
+  if (AL.getNumArgs() == 0) {
+    S.Diag(AL.getLoc(), diag::err_callback_attribute_no_callee)
+        << AL.getRange();
+    return;
+  }
+
+
+  FunctionDecl *FD = D->getAsFunction();
+  assert(FD && "Expected a function declaration!");
+
+
+  SmallVector<int, 8> EncodingIndices;
+  for (unsigned I = 0, E = AL.getNumArgs(); I < E; ++I) {
+    int32_t ArgIdx = getParamFromNameOrIndex(S, FD, AL, I);
+    if (ArgIdx == -1) return;
     EncodingIndices.push_back(ArgIdx);
   }
 
+  bool HasImplicitThisParam = isInstanceMethod(FD);
   int CalleeIdx = EncodingIndices.front();
   // Check if the callee index is proper, thus not "this" and not "unknown".
   // This means the "CalleeIdx" has to be non-negative if "HasImplicitThisParam"

   D->addAttr(::new (Context) AlignValueAttr(TmpAttr));
 }
 
+template <typename AttrType>
+static void handleSimpleIntStringAttribute(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // check the attribute arguments.
+  if (AL.getNumArgs() != 2) {
+    S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1;
+    return;
+  }
+
+  Expr *E = AL.getArgAsExpr(0);
+
+  FunctionDecl *FD = D->getAsFunction();
+  assert(FD && "Expected a function declaration!");
+
+  int32_t ArgIdx = getParamFromNameOrIndex(S, FD, AL, 0);
+  StringRef Str;
+  S.checkStringLiteralArgumentAttr(AL, 1, Str);
+  
+  D->addAttr(::new (S.Context) AttrType(AL.getRange(), S.Context, ArgIdx, Str,
+                                         AL.getAttributeSpellingListIndex()));
+}
+
+template <typename AttrType>
+static void handleSimpleStringAttribute(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // check the attribute arguments.
+  if (AL.getNumArgs() != 1) {
+    S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1;
+    return;
+  }
+
+  StringRef Str;
+  S.checkStringLiteralArgumentAttr(AL, 0, Str);
+  
+  D->addAttr(::new (S.Context) AttrType(AL.getRange(), S.Context, Str,
+                                         AL.getAttributeSpellingListIndex()));
+}
+
 static void handleAlignedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   // check the attribute arguments.
   if (AL.getNumArgs() > 1) {

     S.Diag(AL.getLoc(), diag::err_stmt_attribute_invalid_on_decl)
         << AL << D->getLocation();
     break;
+  case ParsedAttr::AT_LLVMFN:
+    handleSimpleStringAttribute<LLVMFNAttr>(S, D, AL);
+    break;
+  case ParsedAttr::AT_LLVMARG:
+    handleSimpleIntStringAttribute<LLVMARGAttr>(S, D, AL);
+    break;
+  case ParsedAttr::AT_LLVMRET:
+    handleSimpleStringAttribute<LLVMRETAttr>(S, D, AL);
+    break;
   case ParsedAttr::AT_Interrupt:
     handleInterruptAttr(S, D, AL);
     break;

clang/utils/TableGen/ClangAttrEmitter.cpp

     Ptr = llvm::make_unique<VariadicParamIdxArgument>(Arg, Attr);
   else if (ArgName == "VariadicParamOrParamIdxArgument")
     Ptr = llvm::make_unique<VariadicParamOrParamIdxArgument>(Arg, Attr);
+  else if (ArgName == "ParamOrParamIdxArgument")
+    Ptr = llvm::make_unique<SimpleArgument>(Arg, Attr, "int");
   else if (ArgName == "ParamIdxArgument")
     Ptr = llvm::make_unique<SimpleArgument>(Arg, Attr, "ParamIdx");
   else if (ArgName == "VariadicIdentifierArgument")

          llvm::StringSwitch<bool>(
              Arg->getSuperClasses().back().first->getName())
              .Case("VariadicParamOrParamIdxArgument", true)
+             .Case("ParamOrParamIdxArgument", true)
              .Default(false);
 }
 

llvm/include/llvm/IR/Attributes.h

   static Attribute fromRawPointer(void *RawPtr) {
     return Attribute(reinterpret_cast<AttributeImpl*>(RawPtr));
   }
+
+  static Attribute::AttrKind parseAttrKind(StringRef Kind);
 };
 
 // Specialized opaque value conversions.

llvm/lib/IR/Attributes.cpp

   llvm_unreachable("Unknown attribute");
 }
 
+Attribute::AttrKind Attribute::parseAttrKind(StringRef Kind) {
+  return StringSwitch<Attribute::AttrKind>(Kind)
+      .Case("alwaysinline", Attribute::AlwaysInline)
+      .Case("builtin", Attribute::Builtin)
+      .Case("cold", Attribute::Cold)
+      .Case("convergent", Attribute::Convergent)
+      .Case("inlinehint", Attribute::InlineHint)
+      .Case("jumptable", Attribute::JumpTable)
+      .Case("minsize", Attribute::MinSize)
+      .Case("naked", Attribute::Naked)
+      .Case("nobuiltin", Attribute::NoBuiltin)
+      .Case("noduplicate", Attribute::NoDuplicate)
+      .Case("noimplicitfloat", Attribute::NoImplicitFloat)
+      .Case("noinline", Attribute::NoInline)
+      .Case("nonlazybind", Attribute::NonLazyBind)
+      .Case("noredzone", Attribute::NoRedZone)
+      .Case("noreturn", Attribute::NoReturn)
+      .Case("nocf_check", Attribute::NoCfCheck)
+      .Case("norecurse", Attribute::NoRecurse)
+      .Case("nounwind", Attribute::NoUnwind)
+      .Case("optforfuzzing", Attribute::OptForFuzzing)
+      .Case("optnone", Attribute::OptimizeNone)
+      .Case("optsize", Attribute::OptimizeForSize)
+      .Case("readnone", Attribute::ReadNone)
+      .Case("readonly", Attribute::ReadOnly)
+      .Case("argmemonly", Attribute::ArgMemOnly)
+      .Case("returns_twice", Attribute::ReturnsTwice)
+      .Case("safestack", Attribute::SafeStack)
+      .Case("shadowcallstack", Attribute::ShadowCallStack)
+      .Case("sanitize_address", Attribute::SanitizeAddress)
+      .Case("sanitize_hwaddress", Attribute::SanitizeHWAddress)
+      .Case("sanitize_memory", Attribute::SanitizeMemory)
+      .Case("sanitize_thread", Attribute::SanitizeThread)
+      .Case("speculative_load_hardening", Attribute::SpeculativeLoadHardening)
+      .Case("ssp", Attribute::StackProtect)
+      .Case("sspreq", Attribute::StackProtectReq)
+      .Case("sspstrong", Attribute::StackProtectStrong)
+      .Case("strictfp", Attribute::StrictFP)
+      .Case("uwtable", Attribute::UWTable)
+      .Default(Attribute::None);
+}
+
 bool Attribute::operator<(Attribute A) const {
   if (!pImpl && !A.pImpl) return false;
   if (!pImpl) return true;

llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp

                              "example -force-attribute=foo:noinline. This "
                              "option can be specified multiple times."));
 
-static Attribute::AttrKind parseAttrKind(StringRef Kind) {
-  return StringSwitch<Attribute::AttrKind>(Kind)
-      .Case("alwaysinline", Attribute::AlwaysInline)
-      .Case("builtin", Attribute::Builtin)
-      .Case("cold", Attribute::Cold)
-      .Case("convergent", Attribute::Convergent)
-      .Case("inlinehint", Attribute::InlineHint)
-      .Case("jumptable", Attribute::JumpTable)
-      .Case("minsize", Attribute::MinSize)
-      .Case("naked", Attribute::Naked)
-      .Case("nobuiltin", Attribute::NoBuiltin)
-      .Case("noduplicate", Attribute::NoDuplicate)
-      .Case("noimplicitfloat", Attribute::NoImplicitFloat)
-      .Case("noinline", Attribute::NoInline)
-      .Case("nonlazybind", Attribute::NonLazyBind)
-      .Case("noredzone", Attribute::NoRedZone)
-      .Case("noreturn", Attribute::NoReturn)
-      .Case("nocf_check", Attribute::NoCfCheck)
-      .Case("norecurse", Attribute::NoRecurse)
-      .Case("nounwind", Attribute::NoUnwind)
-      .Case("optforfuzzing", Attribute::OptForFuzzing)
-      .Case("optnone", Attribute::OptimizeNone)
-      .Case("optsize", Attribute::OptimizeForSize)
-      .Case("readnone", Attribute::ReadNone)
-      .Case("readonly", Attribute::ReadOnly)
-      .Case("argmemonly", Attribute::ArgMemOnly)
-      .Case("returns_twice", Attribute::ReturnsTwice)
-      .Case("safestack", Attribute::SafeStack)
-      .Case("shadowcallstack", Attribute::ShadowCallStack)
-      .Case("sanitize_address", Attribute::SanitizeAddress)
-      .Case("sanitize_hwaddress", Attribute::SanitizeHWAddress)
-      .Case("sanitize_memory", Attribute::SanitizeMemory)
-      .Case("sanitize_thread", Attribute::SanitizeThread)
-      .Case("speculative_load_hardening", Attribute::SpeculativeLoadHardening)
-      .Case("ssp", Attribute::StackProtect)
-      .Case("sspreq", Attribute::StackProtectReq)
-      .Case("sspstrong", Attribute::StackProtectStrong)
-      .Case("strictfp", Attribute::StrictFP)
-      .Case("uwtable", Attribute::UWTable)
-      .Default(Attribute::None);
-}
-
 /// If F has any forced attributes given on the command line, add them.
 static void addForcedAttributes(Function &F) {
   for (auto &S : ForceAttributes) {

     if (KV.first != F.getName())
       continue;
 
-    auto Kind = parseAttrKind(KV.second);
+    auto Kind = Attribute::parseAttrKind(KV.second);
     if (Kind == Attribute::None) {
       LLVM_DEBUG(dbgs() << "ForcedAttribute: " << KV.second
                         << " unknown or not handled!\n");