> On 10 Jan 2025, at 11:22, Richard Sandiford <richard.sandif...@arm.com> wrote:
>
> <alfie.richa...@arm.com> writes:
>> This patch adds a warning when FMV is used for Aarch64.
>>
>> The reasoning for this is the ACLE [1] spec for FMV has diverged
>> significantly from the current implementation and we want to prevent
>> potential future compatability issues.
>>
>> There is a patch for an ACLE compliant version of target_version and
>> target_clone in progress but it won't make gcc-15.
>>
>> This has been bootstrap and regression tested for Aarch64.
>> Is this okay for master and packport to gcc-14?
>>
>> [1]
>> https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning
>>
>> gcc/ChangeLog:
>>
>> * config/aarch64/aarch64.cc
>> (aarch64_mangle_decl_assembler_name): Add experimental warning.
>> * config/aarch64/aarch64.opt: Add command line option to disable
>> warning.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * g++.target/aarch64/mv-1.C: Add CLI flag
>> * g++.target/aarch64/mv-symbols1.C: Add CLI flag
>> * g++.target/aarch64/mv-symbols2.C: Add CLI flag
>> * g++.target/aarch64/mv-symbols3.C: Add CLI flag
>> * g++.target/aarch64/mv-symbols4.C: Add CLI flag
>> * g++.target/aarch64/mv-symbols5.C: Add CLI flag
>> * g++.target/aarch64/mvc-symbols1.C: Add CLI flag
>> * g++.target/aarch64/mvc-symbols2.C: Add CLI flag
>> * g++.target/aarch64/mvc-symbols3.C: Add CLI flag
>> * g++.target/aarch64/mvc-symbols4.C: Add CLI flag
>> * g++.target/aarch64/mv-warning1.C: New test.
>> ---
>> gcc/config/aarch64/aarch64.cc | 4 ++++
>> gcc/config/aarch64/aarch64.opt | 4 ++++
>> gcc/doc/invoke.texi | 11 ++++++++++-
>> gcc/testsuite/g++.target/aarch64/mv-1.C | 1 +
>> gcc/testsuite/g++.target/aarch64/mv-symbols1.C | 1 +
>> gcc/testsuite/g++.target/aarch64/mv-symbols2.C | 1 +
>> gcc/testsuite/g++.target/aarch64/mv-symbols3.C | 1 +
>> gcc/testsuite/g++.target/aarch64/mv-symbols4.C | 1 +
>> gcc/testsuite/g++.target/aarch64/mv-symbols5.C | 1 +
>> gcc/testsuite/g++.target/aarch64/mv-warning1.C | 9 +++++++++
>> gcc/testsuite/g++.target/aarch64/mvc-symbols1.C | 1 +
>> gcc/testsuite/g++.target/aarch64/mvc-symbols2.C | 1 +
>> gcc/testsuite/g++.target/aarch64/mvc-symbols3.C | 1 +
>> gcc/testsuite/g++.target/aarch64/mvc-symbols4.C | 1 +
>> 14 files changed, 37 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C
>>
>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> index 91de13159cb..7d64e99b76b 100644
>> --- a/gcc/config/aarch64/aarch64.cc
>> +++ b/gcc/config/aarch64/aarch64.cc
>> @@ -20347,6 +20347,10 @@ aarch64_mangle_decl_assembler_name (tree decl, tree
>> id)
>> if (TREE_CODE (decl) == FUNCTION_DECL
>> && DECL_FUNCTION_VERSIONED (decl))
>> {
>> + warning_at (DECL_SOURCE_LOCATION(decl), OPT_Wexperimental_fmv_target,
>> + "Function Multi Versioning support is experimental, and the "
>> + "behavior is likely to change");
>> +
>> aarch64_fmv_feature_mask feature_mask = get_feature_mask_for_version
>> (decl);
>
> Did you consider doing this in aarch64_option_valid_version_attribute_p
> instead? That hook is called directly by the frontend and is something
> that already produces diagnostics for invalid versions.
>
> OK with that change from POV if it works, and if you agree.
> Please give others until Monday to comment though.
I wonder do we want to make it a once-only warning similar to
aarch64_report_sve_required ?
If the user has multiple multiversioned functions in their code maybe warning
only once is okay.
On the other hand, maybe pointing out all of them at once is more helpful.
I don’t feel strongly about it, so happy with either.
Thanks,
Kyrill
>
> Thanks,
> Richard
>
>>
>> std::string name = IDENTIFIER_POINTER (id);
>> diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
>> index 36bc719b822..2a8dd8ea66c 100644
>> --- a/gcc/config/aarch64/aarch64.opt
>> +++ b/gcc/config/aarch64/aarch64.opt
>> @@ -431,3 +431,7 @@ handling. One means we try to form pairs involving one
>> or more existing
>> individual writeback accesses where possible. A value of two means we
>> also try to opportunistically form writeback opportunities by folding in
>> trailing destructive updates of the base register used by a pair.
>> +
>> +Wexperimental-fmv-target
>> +Target Var(warn_experimental_fmv) Warning Init(1)
>> +Warn about usage of experimental Function Multi Versioning.
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 51dc871e6bc..bdf9ee1bc0c 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -822,7 +822,8 @@ Objective-C and Objective-C++ Dialects}.
>> -moverride=@var{string} -mverbose-cost-dump
>> -mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{sysreg}
>> -mstack-protector-guard-offset=@var{offset} -mtrack-speculation
>> --moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion}
>> +-moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion
>> +-Wexperimental-fmv-target}
>>
>> @emph{Adapteva Epiphany Options}
>> @gccoptlist{-mhalf-reg-file -mprefer-short-insn-regs
>> @@ -22087,6 +22088,14 @@ which specify use of that register as a fixed
>> register,
>> and @samp{none}, which means that no register is used for this
>> purpose. The default is @option{-m1reg-none}.
>>
>> +@opindex Wexperimental-fmv-target
>> +@opindex Wno-experimental-fmv-target
>> +@item -Wexperimental-fmv-target
>> +Warn about use of experimental Function Multi Versioning.
>> +The Arm C Language Extension specification for Function Multi Versioning
>> +is beta and subject to change. Any usage of FMV is caveated that future
>> +behavior change and incompatibility is likely.
>> +
>> @end table
>>
>> @node AMD GCN Options
>> diff --git a/gcc/testsuite/g++.target/aarch64/mv-1.C
>> b/gcc/testsuite/g++.target/aarch64/mv-1.C
>> index b4b0e5e3fea..b10037f1b9b 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mv-1.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mv-1.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> __attribute__((target_version("default")))
>> int foo ()
>> diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols1.C
>> b/gcc/testsuite/g++.target/aarch64/mv-symbols1.C
>> index 53e0abcd9b4..73cde42fa34 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mv-symbols1.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mv-symbols1.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> int foo ()
>> {
>> diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols2.C
>> b/gcc/testsuite/g++.target/aarch64/mv-symbols2.C
>> index f0c7967a97a..6da88ddfb48 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mv-symbols2.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mv-symbols2.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> __attribute__((target_version("default")))
>> int foo ()
>> diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols3.C
>> b/gcc/testsuite/g++.target/aarch64/mv-symbols3.C
>> index 3d30e27deb8..5dd7b49be2a 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mv-symbols3.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mv-symbols3.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> __attribute__((target_version("default")))
>> int foo ();
>> diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols4.C
>> b/gcc/testsuite/g++.target/aarch64/mv-symbols4.C
>> index 73e3279ec31..4b25d17cc15 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mv-symbols4.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mv-symbols4.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> __attribute__((target_version("default")))
>> int foo ()
>> diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols5.C
>> b/gcc/testsuite/g++.target/aarch64/mv-symbols5.C
>> index 05d1379f53e..fac00b20313 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mv-symbols5.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mv-symbols5.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> __attribute__((target_version("default")))
>> int foo ();
>> diff --git a/gcc/testsuite/g++.target/aarch64/mv-warning1.C
>> b/gcc/testsuite/g++.target/aarch64/mv-warning1.C
>> new file mode 100644
>> index 00000000000..0addd0c9772
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.target/aarch64/mv-warning1.C
>> @@ -0,0 +1,9 @@
>> +/* { dg-do compile } */
>> +/* { dg-require-ifunc "" } */
>> +/* { dg-options "-O0" } */
>> +
>> +__attribute__((target_version("default")))
>> +int foo () { return 1; }/* { dg-warning "Function Multi Versioning support
>> is experimental, and the behavior is likely to change" } */
>> +
>> +__attribute__((target_version("rng")))
>> +int foo () { return 1; }/* { dg-warning "Function Multi Versioning support
>> is experimental, and the behavior is likely to change" } */
>> diff --git a/gcc/testsuite/g++.target/aarch64/mvc-symbols1.C
>> b/gcc/testsuite/g++.target/aarch64/mvc-symbols1.C
>> index 2dd7c79f16c..983194d74af 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mvc-symbols1.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mvc-symbols1.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> __attribute__((target_clones("default", "dotprod", "sve+sve2")))
>> int foo ()
>> diff --git a/gcc/testsuite/g++.target/aarch64/mvc-symbols2.C
>> b/gcc/testsuite/g++.target/aarch64/mvc-symbols2.C
>> index 75b9c126dd8..58a797947ce 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mvc-symbols2.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mvc-symbols2.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> __attribute__((target_clones("default", "dotprod", "sve+sve2")))
>> int foo ()
>> diff --git a/gcc/testsuite/g++.target/aarch64/mvc-symbols3.C
>> b/gcc/testsuite/g++.target/aarch64/mvc-symbols3.C
>> index 82e777c8fc6..350a5586643 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mvc-symbols3.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mvc-symbols3.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> __attribute__((target_clones("default", "dotprod", "sve+sve2")))
>> int foo ();
>> diff --git a/gcc/testsuite/g++.target/aarch64/mvc-symbols4.C
>> b/gcc/testsuite/g++.target/aarch64/mvc-symbols4.C
>> index 6c86ae61e5f..9c8a7bd37f2 100644
>> --- a/gcc/testsuite/g++.target/aarch64/mvc-symbols4.C
>> +++ b/gcc/testsuite/g++.target/aarch64/mvc-symbols4.C
>> @@ -1,6 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-ifunc "" } */
>> /* { dg-options "-O0" } */
>> +/* { dg-additional-options "-Wno-experimental-fmv-target" } */
>>
>> __attribute__((target_clones("default", "dotprod", "sve+sve2")))
>> int foo ();
