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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to