From: Richard Sandiford <richard.sandiford@arm.com>
To: Andrew Carlotti <andrew.carlotti@arm.com>
Cc: gcc-patches@gcc.gnu.org, Richard Earnshaw <richard.earnshaw@arm.com>
Subject: Re: [PATCH 0/5] aarch64: FMV feature list fixes
Date: Wed, 10 Apr 2024 17:42:05 +0100 [thread overview]
Message-ID: <mpt7ch5tbj6.fsf@arm.com> (raw)
In-Reply-To: <c8c795fd-48b4-38ce-160f-f478b554e5fc@e124511.cambridge.arm.com> (Andrew Carlotti's message of "Wed, 10 Apr 2024 12:10:04 +0100")
Andrew Carlotti <andrew.carlotti@arm.com> writes:
> On Tue, Apr 09, 2024 at 04:43:16PM +0100, Richard Sandiford wrote:
>> Andrew Carlotti <andrew.carlotti@arm.com> writes:
>> > The first three patches are trivial changes to the feature list to reflect
>> > recent changes in the ACLE. Patch 4 removes most of the FMV multiversioning
>> > features that don't work at the moment, and should be entirely uncontroversial.
>> >
>> > Patch 5 handles the remaining cases, where there's an inconsistency in how
>> > features are named in the current FMV specification compared to the existing
>> > command line options. It might be better to instead preserve the "memtag2",
>> > "ssbs2" and "ls64_accdata" names for now; I'd be happy to commit either
>> > version.
>>
>> Yeah, I suppose patch 5 leaves things in a somewhat awkward state,
>> since e.g.:
>>
>> -AARCH64_OPT_FMV_EXTENSION("memtag", MEMTAG, (), (), (), "")
>> +AARCH64_OPT_EXTENSION("memtag", MEMTAG, (), (), (), "")
>>
>> -AARCH64_FMV_FEATURE("memtag2", MEMTAG2, (MEMTAG))
>> +AARCH64_FMV_FEATURE("memtag", MEMTAG2, (MEMTAG))
>>
>> seems to drop "memtag2" and FEAT_MEMTAG, but keep "memtag" and
>> FEAT_MEMTAG2. Is that right?
>
> That's deliberate. The FEAT_MEMTAG bit in __aarch64_cpu_features is defined to
> match the definition of FEAT_MTE in the architecture, and likewise for
> FEAT_MEMTAG2/FEAT_MTE2. However, in Binutils the "+memtag" extension enables
> both FEAT_MTE and FEAT_MTE2 instructions (although none of the FEAT_MTE2
> instructions can be generated from GCC without inline assembly). The FMV
> specification in the ACLE currently uses names "memtag" and "memtag2" that
> match the architecture names, but arguably don't match the command line
> extension names. I'm advocating for that to change to match the extension
> names in command line options.
Hmm, ok. I agree it makes sense for the user-visible FMV namnes to match
the command line. But shouldn't __aarch64_cpu_features either (a) use exactly
the same names as the architecture or (b) use exactly the same names as the
command-line (mangled where necessary)? It seems that we're instead
using a third convention that doesn't exactly match the other two.
That is, I can see the rationale for "memtag" => FEAT_MTE2 and
"memtag" => FEAT_MEMTAG. It just seems odd to have "memtag" => FEAT_MEMTAG2
(where MEMTAG2 is an alias of MTE2).
How much leeway do we have to change the __aarch64_cpu_features names?
Is it supposed to be a public API (as opposed to ABI)?
> The LS64 example is definitely an inconsistency, since GCC uses "+ls64" to
> enable intrinsics for all of the FEAT_LS64/FEAT_LS64_V/FEAT_LS64_ACCDATA
> intrinsics.
Ok, thanks. If we go for option (a) above then I agree that the ls64
change is correct. If we go for option (b) then I suppose it should
stay as LS64.
> There were similar issues with "sha1", "pmull" and "sve2-pmull128", but in
> these cases their presence architecturally is implied by the presence of the
> features checked for "sha2", "aes" and "sve2-aes" so it's fine to just delete
> the ones without command line flags.
>
>> Apart from that and the comment on patch 2, the series looks good to me.
>>
>> While rechecking aarch64-option-extensions.def against the ACLE list:
>> it seems that the .def doesn't treat mops as an FMV feature. Is that
>> deliberate?
>
> "mops" was added to the ACLE list later, and libgcc doesn't yet support
> detecting it. I didn't think it was sensible to add new FMV feature support at
> this stage.
Ah, ok, makes sense.
Richard
next prev parent reply other threads:[~2024-04-10 16:42 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 13:24 Andrew Carlotti
2024-04-09 13:24 ` [PATCH 1/5] aarch64: Reorder FMV feature priorities Andrew Carlotti
2024-04-09 13:25 ` [PATCH 2/5] aarch64: Don't use FEAT_MAX as array length Andrew Carlotti
2024-04-09 15:33 ` Richard Sandiford
2024-04-10 12:33 ` Andrew Carlotti
2024-04-09 13:26 ` [PATCH 3/5] aarch64: Fix typo and make rdma/rdm alias for FMV Andrew Carlotti
2024-04-09 13:26 ` [PATCH 4/5] aarch64: Remove unsupported FMV features Andrew Carlotti
2024-04-09 13:27 ` [PATCH 5/5] aarch64: Combine some " Andrew Carlotti
2024-04-09 15:43 ` [PATCH 0/5] aarch64: FMV feature list fixes Richard Sandiford
2024-04-10 11:10 ` Andrew Carlotti
2024-04-10 16:42 ` Richard Sandiford [this message]
2024-04-10 17:11 ` Andrew Carlotti
2024-04-10 18:51 ` Richard Sandiford
2024-04-10 19:03 ` Andrew Carlotti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=mpt7ch5tbj6.fsf@arm.com \
--to=richard.sandiford@arm.com \
--cc=andrew.carlotti@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=richard.earnshaw@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).