Andrew Carlotti <andrew.carlo...@arm.com> writes:
> On Tue, Apr 09, 2024 at 04:43:16PM +0100, Richard Sandiford wrote:
>> Andrew Carlotti <andrew.carlo...@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
