On 7/21/22 19:49, Daniel Kiss wrote:
> Hello,
>
> Thanks for the quick reply, see mine inline.
>> On 2022. Jul 19., at 12:01, Martin Liška <mli...@suse.cz> wrote:
>>
>> On 7/18/22 12:36, Daniel Kiss via Gcc wrote:
>>> Hello,
>>>
>>> We are going to add Function Multiversioning [1] support to Arm
>>> architectures.
>>> The specification is made public as beta[2] to ensure toolchain that
>>> follows Arm
>>> C Language Extension will implement it in the same way.
>>>
>>> A few tweaks considered to make the developers' life easier.
>>> Since the `target` attribute is used widely on Arm, we would like to
>>> introduce a
>>> new attribute `target_version` to avoid confusion and possible deployment
>>> problems. The `target_clones` attribute will be supported too. Also the
>>> “default”
>>> version to be made optional.
>>>
>>> We are looking for feedback on the specification (reply, github works too).
>>>
>>> Thanks so much,
>>> Daniel
>>>
>>> [1] https://gcc.gnu.org/onlinedocs/gcc/Function-Multiversioning.html
>>> [2]
>>> https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning
>>>
>>
>> Hello.
>>
>> Thanks for working on the feature, it will be nice to cover the gap in
>> between x86_64 and powerpc,
>> which implement the FMV feature.
>>
>> As the person who's been involved with the current MVC code in the GCC, I
>> have a few comments/questions
>> about it:
>>
>> 1) both i386 and Powerpc also allow specifying an equivalent to -march (like
>> `arch=bdver2`),
>> in Arm case it seems to me only individual features are considered
> Arm architecture version is not definite enough in this case because
> certain features are optional on a given versions and may back ported to
> older versions.
> Implementation name of a core also could be misleading in most of the cases.
> And too many out there if
> all implementation is considered not just Arm’s Cortex cores.
> Also the kernel support varies regardless the actual hardware, features can
> be disabled by the firmware/OS.
> I think developers target a given feature instead of a given uarch most cases.
Sure, that makes fully sense to me!
>
>>
>> 2) about 'target_version' attribute - I like the idea as one can have a
>> completely independent
>> function implementation optimized for an ISA;
>> note it's very close to 'target' attribute (supported in C++), where one
>> needs to provide
>> a resolver and have the pretty same functionality (see e.g.
>> gcc/testsuite/g++.target/i386/mv1.C).
>> However, the feature does not work in C and you will have the very same
>> problem with target_version
>> attribute (multiple functions with the same declaration):
>>
>> mv1.c:76:1: error: redefinition of ‘foo’
>> 76 | foo ()
>> | ^~~
> In our clang implementation\prototype such a use case is supported. The goal
> was to able to write like this in C
> /* existing code*/
> extern int foo();
> int foo(){}
> /* addition */
> #ifdef __ARM_FEATURE_FUNCTION_MULTI_VERSIONING
> __attribute__((target_version(“...")))
> int foo(){}
> #endif
I see, so then it's going to require a more work regarding the C front-end.
Maybe we should enable the same way the "target"
attribute for C.
>
> so an existing codebase can be extended without breaking it even for old
> compilers, without heavy checks for attribute support.
Yep.
>
>> 3) If you will implement 'target_version' attribute, I would like to see it
>> available also for the
>> existing targets supporting MVC
> Yes, this is the plan if other target maintainers will accept it.
> IMHO all semantical differences would work for all targets.
Sure!
>>
>> 4) A small note about the mangling, the existing i386 naming scheme looks
>> like:
>>
>> ...
>> _Z3foov.avx2_ssse3
>> ...
>>
>> 5) Can you please define how will you evaluate priorities for a situation
>> where multiple features
>> are used (e.g. dotprod+sm)?
>>
>> Note we face the very same problem on i386, where it's very hard to
>> specify a priority
>> for the family of avx512 features. That said, an optional priority
>> specifier might be possible.
> ACLE provides a table of priorities for given feature and a simple algorithm
> how to choose.
>
> Version where the most features are requested will be picked,
Ok!
> then the one with the highest priority.
> in case of (dotprod+sm, sve) set the dotprod+sm will be selected just because
> it is more specified, even
> sve has higher priority.
>
> We considered the other of the attributes in the source, but that might be
> quite problematic to preserve during
> compilation.
We can start with that and add priorities later if really needed.
>
> A new attribute or variant that provides priority could work too, just so far
> the newer feature usually a better
> choice, and those got higher priority.
>
>>
>> 6) Note that as opposed to i385 and Powerpc, we don't allow a combination of
>> ISA flags for target_clone
>> attribute (like sse2+avx512f).
> Noted, I think in case of Arm it may make sense to support it.
>>
>> 7) FMV may be disabled in compile time by a compiler flag. In this case the
>> default version shall be used.
>>
>> I would like to see the functionality also target agnostic.
> Sure, I agree. the proposed flag is -mno-fmv (-mfmv default on).
> Also maybe the feature indication define
> __ARM_FEATURE_FUNCTION_MULTI_VERSIONING could be just
> __FEATURE_FUNCTION_MULTI_VERSIONING?
I would take a name inspiration from:
https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
so what about something like __HAVE_FUNCTION_MULTI_VERSIONING ?
Cheers,
Martin
>
>>
>> Anyway, looking forward to the Arm implementation.
>> Hope the comments are constructive.
> Thanks, help me a lot.
>
>>
>> Cheers,
>> Martin
>
