[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added a comment.

In D127762#4023803 <https://reviews.llvm.org/D127762#4023803>, @aaron.ballman wrote:

> In D127762#3960906 <https://reviews.llvm.org/D127762#3960906>, @rsandifo-arm wrote:
>
>> Thanks @aaron.ballman for the feedback about spellings.  I've gone with the lower-case names like `__arm_streaming` in what follows, as a plausible strawman.
>>
>> My main concern with keywords is:
>>
>>> This approach means there's plenty of flexibility available to parse the keyword where you think it makes sense.
>>
>> If everyone parses keywords where it makes sense to them personally, for their specific use case, I'm worried that we'll end up with a lot of slightly different rules.  (This tended to happen for pre-standard features, and like you say, is still the case for GNU attribute handling between Clang and GCC, given GCC's poorly-defined rules.)
>
> Ah, I meant more that we have flexibility on what we want the grammar of the extension to be. If other implementations wanted to pick up the functionality, hopefully they'd pick up the same grammar as well.

Yeah, that's also how I was interpreting your comment.  But my point is that, as the definers of this extension, we didn't really come into this needing/wanting custom grammar rules.  The grammar for standard attributes would have been fine for us.  We can't use those because standard attributes aren't allowed to affect semantics, but that's a rule imposed on us by the standard rather than a rule that we're imposing on ourselves.  The grammar rules and the appurtenance rules would have been fine.

In other words, I agree that we have the flexibility to define specific rules for this extension that do whatever we want.  But I don't think the extension is unusual enough to need that flexibility.  There doesn't seem to be anything about the semantic function type properties in this extension that would require different grammar rules from other semantic function type properties.  It seems better to have a generic way of attaching target-specific semantic information to types (and objects), just like the standard provides a generic way of attaching target-specific non-semantic information.

>> For example, a type property like `__arm_streaming` only applies to functions, so it wouldn't make sense for bespoke rules to allow the keyword in tagged types or in array types.  If a property only applies to object types then it wouldn't make sense for bespoke rules to allow the keyword after a parameter list.
>
> Agreed, but that's a semantic property of the attribute rather than a syntactic property. We have control over both, of course, because this is our extension and we can define it how we like. But I was expecting we'd define a syntactic location to parse the attribute and we'd handle appertainance issues in SemaDeclAttr.cpp when deciding whether to convert the parsed attribute into a semantic attribute or not.

Like you say, I think we're in agreement here, but approaching it from different angles.  What I meant is that, if we add grammar rules that apply only to the keywords defined in this extension, those grammar rules would probably not allow the keyword to appear in things like tagged types or array types, because they would never be needed there.  If each extension defines its own grammar rules, those rules would generally not allow keywords to appear in places where the keywords are always semantically invalid (although see below).  This reduces the extent to which bespoke grammar rules for one extension can be cut-&-pasted into another extension.

Having grammar rules that are defined robotically based on the standard attribute grammar rules, rather than trying to tailor the grammar rules specifically to SME, should make it much more likely that the same grammar rules can be reused by later extensions.  Doing that moves all of the extension-specific constraint checking to semantic analysis, rather than splitting it between parsing and semantic analysis.

>> So I think it makes sense to try to make the SME attributes an instance of a (new) generic solution.  I think we want something “like standard attributes, but for semantic information”.  That might sound like wanting something “like ice, but not cold”.  But a lot of valuable work went into defining the parsing rules for standard attributes, and defining what they appertain to.  It seems like a good idea to reuse those parts rather than reinvent the wheel.
>>
>> https://reviews.llvm.org/D139028 is an RFC for adding “attribute-like keywords”: keywords that can appear exactly where a standard attribute can appear.  This means that the keywords are syntactically valid in far more places than necessary for this initial use case, but it provides a simple and mechanical rule.  And the keywords would have to be rejected in those additional places anyway, even if we don't parse them as attributes.
>
> FWIW, we already have a number of attributes implemented via a keyword today: `ConstInitAttr`, `C11NoReturnAttr`, a ton of CUDA and OpenCL keywords, calling conventions like `__stdcall`, etc. I'll take a look at the other patch to see what it's all about, but currently all keyword attributes need explicit parsing support added for them, as in: https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseDecl.cpp#L4005 and https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDecl.cpp#L7767.

Thanks for looking at the patch.  And yeah, I've seen those other keywords, but I think they show examples of the points I was making above.  For example, we have:

  // Microsoft compatibility
  case tok::kw___cdecl:      // struct foo {...} __cdecl      x;
  case tok::kw___fastcall:   // struct foo {...} __fastcall   x;
  case tok::kw___stdcall:    // struct foo {...} __stdcall    x;
  case tok::kw___thiscall:   // struct foo {...} __thiscall   x;
  case tok::kw___vectorcall: // struct foo {...} __vectorcall x;
    // We will diagnose these calling-convention specifiers on non-function
    // declarations later, so claim they are valid after a type specifier.
    return getLangOpts().MicrosoftExt;

This seems like a case where Microsoft defined grammar rules that allow the keywords in more places than strictly necessary, so that the job of rejecting them moves from parsing to semantic analysis.  The grammar rules could instead have rejected the keywords in these locations.  Both ways work, and as far as I can tell, there are no particular differences between the Microsoft extensions and the SME extensions that would suggest the SME extensions should take a different approach.  So we could accept the SME keywords in the same places “for consistency”, or we could reject them on the basis that the keywords are never needed in that context (just like they aren't needed in that context for the Microsoft extension).  The choice seems pretty arbitrary.

I think there's a danger that adding SME in a similar ad hoc way would increase the sense of arbitrariness for whoever adds the next similar extension. :-)

>> The covering note has a lot more justification (too much, sorry!).  Does this look like a possible way forward?
>>
>> Previously I was arguing in favour of the decl-to-type sliding rule, because:
>>
>>> From discussions I've had with people who will write/are writing SME code, the distinction between arm_locally_streamng being a property of the definition and arm_streaming being a property of the type seems contrived to them, especially when the usage of SME is private to a single translation unit.
>>
>> Taking the RFC approach involves abandoning that and sticking strictly to the standard appurtenance rules.  But hopefully it's a reasonable trade-off.  The sliding rule is clearly going against the direction of travel anyway, so I'm probably being too Luddite there.
>
> Err, I'm struggling to see why the distinction is contrived. If the attribute applies to the type, then you get type-based semantics like the ability to overload or specialize on the presence/absence of the attribute -- you shouldn't get that behavior from a declaration attribute. e.g., if you use a declaration attribute on a function definition and then redefine the function without the attribute, that should give a redefinition error because that's two functions with the same name and same type being defined.

Yeah, I agree it's not contrived from a compiler developer's point of view, or from the point of view of someone with a specific interesting in language semantics.  But the users I talked to are experts in other fields who just use C++ as a tool.  The common use case for them was to write a function with certain properties and then call it.  From that basic use case, it doesn't matter (to them) whether certain properties appertain to the types and others to the declaration.

The difference would start to matter to them if they defined virtual functions (for example), or exported the SME-specific functions as an API.  But that wasn't the use case they were considering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127762