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

bryanpkc added a comment. @sdesmalen @rsandifo-arm @aaron.ballman @erichkeane What is the plan for this patch? Is it going to be abandoned in favor of D139028 , or can it be landed in its current form after addressing all outstanding review comments?

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

aaron.ballman added a comment. In D127762#4102578 , @erichkeane wrote: > By standard imp-limits, apparently we only need to handle 8 bits worth for > the Exception Spec types. I'd be tempted, based on exception specs being > deprecated and rarely

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

erichkeane added a comment. In D127762#4102526 , @aaron.ballman wrote: > In D127762#4102436 , @erichkeane > wrote: > >> Finally, and this is something @aaron.ballman probably wants to answer: Is >> this

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

aaron.ballman added a comment. In D127762#4102436 , @erichkeane wrote: > Finally, and this is something @aaron.ballman probably wants to answer: Is > this sufficiently important that we're willing to take the additional > overhead of 8 bits for each

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

erichkeane added a comment. So I still don't see this doing the module writing/reading part, which is necessary if you're making this part of the type. I'd like to see more testing/thought put into how this interfaces with overloads and template resolution. Finally, and this is something

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

aemerson added a comment. Gentle ping on this discussion. @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127762/new/ https://reviews.llvm.org/D127762 ___ cfe-commits mailing list

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

rsandifo-arm added a comment. In D127762#4023803 , @aaron.ballman wrote: > In D127762#3960906 , @rsandifo-arm > wrote: > >> Thanks @aaron.ballman for the feedback about spellings. I've gone with the >>

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

aaron.ballman added a comment. In 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

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

sdesmalen updated this revision to Diff 482440. sdesmalen added a comment. Changed arm_new_za to be a declaration attribute instead of a type attribute (this was something that @rsandifo-arm pointed out to me recently) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

rsandifo-arm added a comment. 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

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

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Type.h:4008 bool HasTrailingReturn : 1; +unsigned AArch64SMEAttributes : 8; Qualifiers TypeQuals; sdesmalen wrote: > aaron.ballman wrote: > > erichkeane wrote: > > > sdesmalen

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

sdesmalen added inline comments. Comment at: clang/include/clang/AST/Type.h:3940 +/// on declarations and function pointers. +unsigned AArch64SMEAttributes : 8; + erichkeane wrote: > sdesmalen wrote: > > erichkeane wrote: > > > We seem to be missing all

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

sdesmalen updated this revision to Diff 477116. sdesmalen marked 10 inline comments as done. sdesmalen added a comment. Addressed review comments. These include: - Allow dropping the arm_preserves_za attribute. - Added tests for serializing/deserializing the AST. - Changed descriptions in

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

rsandifo-arm added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2000 "overridden virtual function is here">; +def err_conflicting_overriding_attributes : Error< + "virtual function %0 has different attributes " aaron.ballman

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

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2000 "overridden virtual function is here">; +def err_conflicting_overriding_attributes : Error< + "virtual function %0 has different attributes " rsandifo-arm

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

rsandifo-arm added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2000 "overridden virtual function is here">; +def err_conflicting_overriding_attributes : Error< + "virtual function %0 has different attributes " rsandifo-arm

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

rsandifo-arm added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2000 "overridden virtual function is here">; +def err_conflicting_overriding_attributes : Error< + "virtual function %0 has different attributes " aaron.ballman

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

aaron.ballman added a comment. In D127762#3930988 , @paulwalker-arm wrote: > Hi @rsandifo-arm , what are your thoughts on Arron's observations? My > interpretation is that Arm originally figured the distinction between > keywords and gnu attributes

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

paulwalker-arm added a comment. Hi @rsandifo-arm , what are your thoughts on Arron's observations? My interpretation is that Arm originally figured the distinction between keywords and gnu attributes was minimal and thus using our previous norms made most sense. This is not my world so my

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

aaron.ballman added a comment. At some point, you should also add a release note to tell users about the new functionality. Comment at: clang/include/clang/AST/Type.h:4008 bool HasTrailingReturn : 1; +unsigned AArch64SMEAttributes : 8; Qualifiers TypeQuals;

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

erichkeane added a comment. I also don't see anything in this with how it works with function pointer conversions, how it affects template instantiation, overload resolution/etc. If this is a type attribute, we need to specify all of that, probably in a separate document, since this is such a

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

sdesmalen added inline comments. Comment at: clang/include/clang/AST/Type.h:3940 +/// on declarations and function pointers. +unsigned AArch64SMEAttributes : 8; + erichkeane wrote: > We seem to be missing all of the modules-storage code for these. Since

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

erichkeane added inline comments. Comment at: clang/include/clang/AST/Type.h:3926 +SME_AttributeMask = 255 // We only support maximum 8 bits because of the +// bitmask in FunctionTypeExtraBitfields + };

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

aaron.ballman added a reviewer: erichkeane. aaron.ballman added a comment. Adding Erich as the new attributes code owner. Comment at: clang/include/clang/Basic/Attr.td:2322 +def ArmStreamingCompatible : DeclOrTypeAttr, TargetSpecificAttr { + let Spellings =

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

sdesmalen added a comment. Gentle ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127762/new/ https://reviews.llvm.org/D127762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

rsandifo-arm added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2000 "overridden virtual function is here">; +def err_conflicting_overriding_attributes : Error< + "virtual function %0 has different attributes " sdesmalen

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

sdesmalen added a subscriber: rsandifo-arm. sdesmalen added a comment. The past few months we've worked to get the attributes at the LLVM IR side implemented. Since that work has now landed, this patch should no longer be held up by the LLVM side of things. @aaron.ballman I've updated and

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

sdesmalen updated this revision to Diff 471515. sdesmalen marked 9 inline comments as done. sdesmalen edited the summary of this revision. sdesmalen added a comment. - Rebased patch - Use `InheritableAttr` for the `ArmLocallyStreaming` attribute instead of `DeclOrTypeAttr` - Added test-case to

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

aaron.ballman added a comment. In D127762#3615702 , @sdesmalen wrote: > Thanks for your patience reviewing this patch @aaron.ballman! Happy to help, thank you for your patience with my constant stream of questions about the design. :-)

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

sdesmalen added a comment. Thanks for your patience reviewing this patch @aaron.ballman! Comment at: clang/include/clang/AST/Type.h:4064 bool HasTrailingReturn : 1; +unsigned AArch64SMEAttributes : 8; Qualifiers TypeQuals; aaron.ballman wrote: >

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

sdesmalen updated this revision to Diff 440631. sdesmalen marked 3 inline comments as done. sdesmalen added a comment. - Removed attribute handling from SemaDeclAttr.cpp, so that the type attributes are only handled in SemaType.cpp - Made ArmLocallyStreaming a DeclOrTypeAttribute again. - Added

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

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Type.h:4064 bool HasTrailingReturn : 1; +unsigned AArch64SMEAttributes : 8; Qualifiers TypeQuals; If we're taking up space in the extra bitfields, why do we need to also take

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

sdesmalen added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2322 +def ArmStreamingCompatible : DeclOrTypeAttr, TargetSpecificAttr { + let Spellings = [Clang<"arm_streaming_compatible">]; aaron.ballman wrote: > sdesmalen wrote: > >

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

sdesmalen updated this revision to Diff 439330. sdesmalen marked 7 inline comments as done. sdesmalen added a comment. - Limited attribute to GNU spelling __attribute__((...)) - Changed `arm_locally_streaming` attribute to be `DeclOrStmtAttr` because it does not apply to type (only the

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

aaron.ballman added a comment. In D127762#3589347 , @sdesmalen wrote: > Thanks @aaron.ballman for your elaborate review, that was very helpful! I'm > still working through some of your suggestions (although some of them weren't > entirely clear to me),

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

sdesmalen updated this revision to Diff 438624. sdesmalen added a comment. - Increased test-coverage by adding positive tests for: - template instantiations - function overloading - lambda function with attribute - (indirect) pointer to pointer to an attributed function type. - Also

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

sdesmalen added a comment. Thanks @aaron.ballman for your elaborate review, that was very helpful! I'm still working through some of your suggestions (although some of them weren't entirely clear to me), but I've addressed a number of them already. In D127762#3582753

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

sdesmalen updated this revision to Diff 437580. sdesmalen marked 10 inline comments as done. sdesmalen added a comment. Addressed bunch of the review comments (though not all yet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127762/new/

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

aaron.ballman added a comment. > This patch adds all the language-level function attributes defined in: > > https://github.com/ARM-software/acle/pull/188 This pull request has not yet been merged, so what are the chances that it gets rejected or altered? > LLVM support for these attributes

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

sdesmalen created this revision. sdesmalen added reviewers: paulwalker-arm, aaron.ballman, aemerson, t.p.northover. Herald added subscribers: jdoerfert, kristof.beyls. Herald added a project: All. sdesmalen requested review of this revision. Herald added a project: clang. Herald added a