User Details
- User Since
- May 1 2018, 4:00 AM (282 w, 6 d)
Yesterday
The requirement exists for streaming-compatible calls to normal functions.
Aug 7 2023
LGTM in terms of intent.
LGTM too, thanks.
The intent looks good to me, but I'm not in a position to approve. Very pedantic, sorry, but on the description, I think:
Superceded by D148700
Aug 3 2023
Thanks for dropping the attribute requirements down to TargetAArch64. But the offsetting requirement was that something somewhere (CodeGen?) must raise an error if code requires SME and SME isn't present. I think that means:
Aug 2 2023
Mostly LGTM from a spec POV, but some comments below.
Jul 12 2023
Jun 13 2023
Do you have any more thoughts on the above? Quick version is:
- Is it OK to have [[…]] attributes in the arm namespace that affect semantics?
- Is it OK to raise an error for unrecognised attributes in the arm namespace (for a measure of future-proofing)?
Jun 7 2023
May 31 2023
Update base commit.
Rebase to account for a diagnostic that has moved.
No other changes from the previous version.
May 30 2023
Avoid most uses of << 0. Add comments to those that remain.
May 25 2023
Thanks @aaron.ballman and @erichkeane for your patience in reviewing the patch and steering me in the right direction.
May 23 2023
Update onto current main. Only change is to replace C++2b with C++23.
Add internals documentation. Add FIXME to temporary code.
Hi @erichkeane and @aaron.ballman. Does the updated patch look OK?
May 16 2023
Thanks @erichkeane . Adding the documentation with that kind of disclaimer sounds good to me. I've done that in the updated version, and also fixed the comment problem that Aaron pointed out.
Add documentation. Fix a comment cut-&-pasto that Aaron pointed out.
Hi @aaron.ballman and @erichkeane . Do you have any more thoughts on this? In principle, I'm happy to convert an existing attribute over to the new scheme, but in practice, I can't find one that seems to be suitable. If we're going to do that, I think I'll need guidance as to which attribute to convert.
May 10 2023
Thanks for the review.
May 8 2023
Apr 26 2023
Apr 25 2023
Apr 19 2023
Add #ifndef guard.
Make AttrTokenKinds.inc use a new KEYWORD_ATTRIBUTE macro, rather than
using KEYWORD directly. Move the #undef to the .inc file.
Thanks for the reviews!
See https://reviews.llvm.org/D139028 for previous discussion on this topic. https://reviews.llvm.org/D148700 contains the Attr.td support (and the main rationale) while https://reviews.llvm.org/D148702 contains the main Parse & Sema support.
Apr 13 2023
Apr 12 2023
Tweak initializer to avoid an MSVC error: https://godbolt.org/z/sMxsW8G8j
Gah, sorry. Due to a botched git operation, I posted a first cut with a stupid typo, rather than the version that passed testing.
FWIW, the original reason for looking at this was to explore ways of removing AS_AttributeLikeKeyword from https://reviews.llvm.org/D139028 . I wanted to find a way of conveying extra information about a spelling without having to change the existing enums. It seemed like that was essentially the same problem as the one described in the FIXME being fixed here.
Apr 10 2023
I threw together a patch to make the constructors explicit and the only two compile failures I have are with what you're fixing in this patch. If you'd like, I can commandeer this patch and subsume it with the larger refactor. Alternatively, we can land this (I'd drop the test though) and I can rebase on top of your changes. Either is fine by me.
Apr 6 2023
Yeah. Is there a way to force the syntax and spelling fields to be dumped, even for implicit attributes? I was struggling to find one, which is why I wasn't sure this made a difference in practice.
Add AST dump test
Apr 5 2023
Mar 31 2023
Feb 28 2023
Thanks @aaron.ballman and @erichkeane for the comments. I'll split it into stages and use tablegen more, like you say.
Jan 26 2023
I'm just responding to the above without having full context, so sorry if this is rehashing old discussions.
Jan 25 2023
Ping. If this approach is acceptable, I think we could use it for future ACLE features too (rather than the GNU attributes that we've used previously).
Jan 4 2023
Reupload with proper linting. No change in code.
Nov 30 2022
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.
Nov 22 2022
Nov 16 2022
Oct 28 2022
Oct 14 2022
Thanks for the patch. This is going to be inconvenient, sorry, but: while implementing the specification in GCC, I noticed that the ZA functions weren't consistent about whether they had an _m suffix. svwrite (MOVA) had one, but the MOP intrinsics that you're implementing here didn't. Since SME2 does have some unpredicated instructions, it seems like it would be better to make the MOP intrinsics consistent with svwrite, with an _m suffix.
Sep 8 2022
I can see at least three ways of handling this:
- @paulwalker-arm's suggestion to allow -march= without a base architecture (or with a dummy base architecture that means “no change”)
- a single new option -mfoo= that applies on top of -march and can be used to add or remove architecture features
- individual -m and -mno- options for each ISA feature.
Jul 5 2022
Mar 16 2022
This approach looks good to me FWIW. The main advantages as I see it are:
- Treating PSTATE.SM and PSTATE.ZA as orthogonal to the set of implemented features follows the scheme used in the ISA specification, so it's easy to cross-check the two. The IsSVEEnabled review comment is an example of this.
- It is likely to cope better with any future extensions to SVE or SME: the streaming-compatible, streaming and non-streaming subsets of each ISA extension would not need to be identified individually on the command line or in IR. Or to put it another way: requiring multiple +streaming-foo flags (and so repeating the “streaming”) feels like specifying the same bit of information multiple times.
- It avoids the risk of combinatorial explosion if we ever need to do something similar for another degree of freedom.
Oct 19 2021
Sorry to ask about an already-closed ticket, but: it looks like this makes the register unconditionally available, rather than making it conditional on FeaturePerfMon. Is that the intended behaviour? GNU as does still require v8.4 for this register.