Hi Alfie,
> On 9 Jan 2025, at 10:58, alfie.richa...@arm.com wrote:
>
> This patch adds a warning whenever 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
> future compatability issues.
>
> There is a patch for and ACLE compliant version of target_version and
> target_clone coming eventually 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 | 3 +++
> gcc/config/aarch64/aarch64.opt | 4 ++++
> 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 +
> 13 files changed, 26 insertions(+)
> create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C
>
> <0001-Add-warning-for-use-of-non-spec-FMV-in-Aarch64.patch>
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 91de13159cb..afc0749fd67 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -20347,6 +20347,9 @@ 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");
+
Some wording nits.
Space before the “(“.
I think there should be no ‘-‘ here to keep consistent with the ACLE wording.
Not sure whether Function Multi Versioning should be capitalised. What do you
think?
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 36bc719b822..55670eeb74f 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
Should this have aarch64 in the name somehow? It feels awkward to have aarch64
in the name, but the option is not generic.
In any case, this should be documented in invoke.texi.
I also think that from a user experience POV if they get this warning they may
ask what the recommendation is.
Should they change their source? Be prepared that the spec will change in
future releases??
The documentation should give some guidance
Thanks,
Kyrill
