[PATCH] D148700: [clang] Add support for “regular” keyword attributes

Wed Jun 7 02:38:01 PDT 2023

rsandifo-arm added a comment.

In D148700#4401451 <https://reviews.llvm.org/D148700#4401451>, @rsmith wrote:

> In D148700#4401353 <https://reviews.llvm.org/D148700#4401353>, @jyknight wrote:
>
>> Yes, standard attributes aren't supposed to be used for things which affect the type system (although, we certainly have many, already, which do, since we expose most GCC-syntax attributes also as C++-standard attribute syntax!)
>
> The rule that standard attributes don't affect semantics only applies to attributes specified by the language standard. There is no expectation that vendor attributes avoid such effects.

Ah.  Does that mean that, when the standard says (sorry for the direct latex quote):

> For an \grammarterm{attribute-token} (including an \grammarterm{attribute-scoped-token}) not specified in this document, the behavior is \impldef{behavior of non-standard attributes}; any such \grammarterm{attribute-token} that is not recognized by the implementation is ignored.

the “is ignored” rule only applies to unscoped attributes and to attributes in one of the std:: namespaces?  I.e. to things that could reasonably be standardised in future, rather than to vendor attributes?

> In particular, I'm concerned by this in the description of this change:
>
> In D148700 <https://reviews.llvm.org/D148700>, @rsandifo-arm wrote:
>
>> Really, the “only” thing wrong with using standard attributes is that standard attributes cannot affect semantics.
>
> If the only reason for this patch series is an idea that vendor attributes using `[[...]]` syntax can't affect program semantics, then I think this change is not justified, because vendor attributes using `[[...]]` syntax can and usually do affect program semantics. But the documentation change here makes the point that using a keyword attribute may be as good idea in cases where you would always want compilation to fail on a compiler that doesn't understand the annotation, rather than the annotation being ignored (likely with a warning), so maybe that's reasonable justification for this direction.

FWIW, the original plan had been to use standard attribute syntax with the `arm` vendor namespace.  We started out with things like `[[arm::streaming]]`, but then a colleague pointed out that standard attributes aren't supposed to affect semantics.  So then we switched to GNU attributes, which have long included things that can't be ignored (such as `vector_size`).  But there was pushback against that because even unrecognised GNU attributes only generate a warning.  (I think GCC made a mistake by establishing that unrecognised GNU attributes are only a warning.)

If using standard attributes with a vendor namespace is acceptable for things that affect semantics and ABI, then that would actually be more convenient, for a few reasons:

- It would provide proper scoping (via namespaces), rather than the clunky `__arm_` prefixes.  E.g. some of the ACLE intrinsics have `__arm_streaming __arm_shared_za __arm_preserves_za`, which is quite a mouthful :)

- It would allow other attribute-related features to be used, such as `#pragma clang attribute`

- It would allow attributes to take arguments, without any compatibility concerns.  Keyword attributes could support arguments, but the decision about whether an attribute takes arguments would probably have to be made when the attribute is first added.  Trying to retrofit arguments to a keyword attribute that didn't previously take arguments could lead to parsing ambiguities.  In contrast, the standard attribute syntax would allow optional arguments to be added later without any backward compatibility concerns.

The main drawback would still be that unrecognised attributes only generate a warning.  But if vendor attributes are allowed to affect semantics, we could “solve” the warning problem for current and future compilers by reporting an error for unrecognised `arm::` attributes.  That wouldn't help with older compilers, but TBH, I think older compilers would reject any practical use of SME attributes anyway.  E.g. the attributes would almost always (perhaps always) be used in conjunction with the `arm_sme.h` header, which older compilers don't provide.  We also provide a feature preprocessor macro that careful code can check.

So if using an `arm` namespace is acceptable, if the current line about what can affect semantics is likely to become more fuzzy, and if there's a risk that keyword attributes are a dead end that no-one else adopts, then TBH I'd still be in favour of using `arm` namespaces for SME.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148700