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.
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.
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.
> Thanks,
> Richard
