Re: [WIP RFC] Add support for keyword-based attributes

[Richard Sandiford <richard.sandiford@arm.com>]

Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, Jul 14, 2023 at 5:58 PM Richard Sandiford via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> Summary: We'd like to be able to specify some attributes using
>> keywords, rather than the traditional __attribute__ or [[...]]
>> syntax.  Would that be OK?
>>
>> In more detail:
>>
>> We'd like to add some new target-specific attributes for Arm SME.
>> These attributes affect semantics and code generation and so they
>> can't simply be ignored.
>>
>> Traditionally we've done this kind of thing by adding GNU attributes,
>> via TARGET_ATTRIBUTE_TABLE in GCC's case.  The problem is that both
>> GCC and Clang have traditionally only warned about unrecognised GNU
>> attributes, rather than raising an error.  Older compilers might
>> therefore be able to look past some uses of the new attributes and
>> still produce object code, even though that object code is almost
>> certainly going to be wrong.  (The compilers will also emit a default-on
>> warning, but that might go unnoticed when building a big project.)
>>
>> There are some existing attributes that similarly affect semantics
>> in ways that cannot be ignored.  vector_size is one obvious example.
>> But that doesn't make it a good thing. :)
>>
>> Also, C++ says this for standard [[...]] attributes:
>>
>>   For an attribute-token (including an attribute-scoped-token)
>>   not specified in this document, the behavior is implementation-defined;
>>   any such attribute-token that is not recognized by the implementation
>>   is ignored.
>>
>> which doubles down on the idea that attributes should not be used
>> for necessary semantic information.
>>
>> One of the attributes we'd like to add provides a new way of compiling
>> existing code.  The attribute doesn't require SME to be available;
>> it just says that the code must be compiled so that it can run in either
>> of two modes.  This is probably the most dangerous attribute of the set,
>> since compilers that ignore it would just produce normal code.  That
>> code might work in some test scenarios, but it would fail in others.
>>
>> The feeling from the Clang community was therefore that these SME
>> attributes should use keywords instead, so that the keywords trigger
>> an error with older compilers.
>>
>> However, it seemed wrong to define new SME-specific grammar rules,
>> since the underlying problem is pretty generic.  We therefore
>> proposed having a type of keyword that can appear exactly where
>> a standard [[...]] attribute can appear and that appertains to
>> exactly what a standard [[...]] attribute would appertain to.
>> No divergence or cherry-picking is allowed.
>>
>> For example:
>>
>>   [[arm::foo]]
>>
>> would become:
>>
>>   __arm_foo
>>
>> and:
>>
>>   [[arm::bar(args)]]
>>
>> would become:
>>
>>   __arm_bar(args)
>>
>> It wouldn't be possible to retrofit arguments to a keyword that
>> previously didn't take arguments, since that could lead to parsing
>> ambiguities.  So when a keyword is first added, a binding decision
>> would need to be made whether the keyword always takes arguments
>> or is always standalone.
>>
>> For that reason, empty argument lists are allowed for keywords,
>> even though they're not allowed for [[...]] attributes.
>>
>> The argument-less version was accepted into Clang, and I have a follow-on
>> patch for handling arguments.  Would the same thing be OK for GCC,
>> in both the C and C++ frontends?
>>
>> The patch below is a proof of concept for the C frontend.  It doesn't
>> bootstrap due to warnings about uninitialised fields.  And it doesn't
>> have tests.  But I did test it locally with various combinations of
>> attribute_spec and it seemed to work as expected.
>>
>> The impact on the C frontend seems to be pretty small.  It looks like
>> the impact on the C++ frontend would be a bit bigger, but not much.
>>
>> The patch contains a logically unrelated change: c-common.h set aside
>> 16 keywords for address spaces, but of the in-tree ports, the maximum
>> number of keywords used is 6 (for amdgcn).  The patch therefore changes
>> the limit to 8 and uses 8 keywords for the new attributes.  This keeps
>> the number of reserved ids <= 256.
>
> If you had added __arm(bar(args)) instead of __arm_bar(args) you would only
> need one additional keyword - we could set aside a similar one for each
> target then.  I realize that double-nesting of arguments might prove a bit
> challenging but still.

Yeah, that would work.

> In any case I also think that attributes are what you want and their
> ugliness/issues are not worse than the ugliness/issues of the keyword
> approach IMHO.

I guess the ugliness of keywords is the double underscore?
What are the issues with the keyword approach though?

If it's two underscores vs miscompilation then it's not obvious
that two underscores should lose.

Richard