Andrew Carlotti <andrew.carlo...@arm.com> writes:
> On Wed, Apr 10, 2024 at 05:42:05PM +0100, Richard Sandiford wrote:
>> 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.
>
> I agree that the name isn't one I would choose now, but I don't think it
> matters much that it's inconsistent.
I kind-of think it does though. Given...
>> 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)?
>
> I think we're designing it to be capable of being a public API, but we haven't
> yet made it one. That's partly why I've kept the enum value names the same as
> in LLVM so far.
...this, I don't want to sleep-walk into a situation where we have
one naming convention for the architecture, one for the attributes,
and a third one for the API. If we're not in a position to commit
to a consistent naming scheme for the API by GCC 14 then it might be
better to remove the FMV features in 5/5 for GCC 14 and revisit in GCC 15.
A patch to do that is pre-approved if you agree (but please say
if you don't).
Thanks,
Richard
