<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_process_target_version_attr): 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.
> * g++.target/aarch64/mvc-warning1.C: New test.
>
> ---
> gcc/config/aarch64/aarch64.cc | 9 +++++++++
> 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 +
> gcc/testsuite/g++.target/aarch64/mvc-warning1.C | 6 ++++++
> 15 files changed, 48 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C
> create mode 100644 gcc/testsuite/g++.target/aarch64/mvc-warning1.C
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 91de13159cb..eae184c7d65 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -20125,6 +20125,15 @@ aarch64_parse_fmv_features (const char *str,
> aarch64_feature_flags *isa_flags,
> static bool
> aarch64_process_target_version_attr (tree args)
> {
> + static bool issued_warning = false;
> + if (!issued_warning)
> + {
> + warning (OPT_Wexperimental_fmv_target,
> + "Function Multi Versioning support is experimental, and the "
> + "behavior is likely to change");
> + issued_warning = true;
> + }
> +
> if (TREE_CODE (args) == TREE_LIST)
> {
> if (TREE_CHAIN (args))
> 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.
> +
It looks like this is adding the option to the Epiphany section. :)
LGTM otherwise, so OK from my POV with that fixed. Please give Kyrill
chance to comment too though.
While I remember: for backward compatibility reasons, we'll need
to make this a dummy option once FMV is no longer experimental,
rather than remove it. (Can't remember if anyone already said that.)
Thanks,
Richard
> @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..6346c1673a2
> --- /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; }
> 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 ();
> diff --git a/gcc/testsuite/g++.target/aarch64/mvc-warning1.C
> b/gcc/testsuite/g++.target/aarch64/mvc-warning1.C
> new file mode 100644
> index 00000000000..59acec3fad2
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/aarch64/mvc-warning1.C
> @@ -0,0 +1,6 @@
> +/* { dg-do compile } */
> +/* { dg-require-ifunc "" } */
> +/* { dg-options "-O0" } */
> +
> +__attribute__((target_clones("default", "sve")))
> +int foo () { return 1; }/* { dg-warning "Function Multi Versioning support
> is experimental, and the behavior is likely to change" } */
