On Fri, Jan 09, 2026 at 10:30:11AM +0000, Richard Earnshaw (foss) wrote:
> On 08/01/2026 17:15, Christophe Lyon wrote:
> >
> >
> > Le jeu. 8 janv. 2026, 17:26, Artemiy Volkov <[email protected]
> > <mailto:[email protected]>> a écrit :
> >
> > On Thu, Jan 08, 2026 at 03:19:47PM +0000, Richard Earnshaw (foss) wrote:
> > > On 08/01/2026 12:35, Christophe Lyon wrote:
> > > >
> > > >
> > > > On 1/8/26 12:53, Artemiy Volkov wrote:
> > > >> The check_effective_target_arm_v8_3a_fp16_complex_neon_ok_nocache
> > > >> procedure in target-supports.exp should return a set of flags
> > providing
> > > >> all of AdvSIMD, complex numbers, and fp16 capabilities.� However,
> > to
> > > >> achieve this we have to be able to overwrite the current -mfpu
> > setting.
> > > >> This means that the current empty string alternative for $flags
> > does not
> > > >> work for us.� This patch splits sets of options to iterate over
> > > >> between the arm and the aarch64 backend and prohibits an empty
> > string for
> > > >> the former.� Moreover, it changes
> > > >> check_effective_target_arm_v8_3a_complex_neon_ok_nocache in the
> > same way.
> > > >>
> > > >> This (fully) fixes the advsimd-intrinsics.exp/vector-complex_f16.c
> > > >> testcase, which was previously being compiled with the
> > -mfpu=neon-fp16
> > > >> flag added by the .exp file itself.
> > > >>
> > > >> Tested on aarch64 by me and on arm by Christophe.
> > > >>
> > > >> Co-authored-by: Richard Earnshaw <[email protected]
> > <mailto:[email protected]>>
> > > >>
> > > >> gcc/testsuite/ChangeLog:
> > > >>
> > > >> ����* lib/target-supports.exp:
> > > >> ����(check_effective_target_arm_v8_3a_complex_neon_ok_nocache):
> > > >> ����Split and fill in arm and aarch64 compile options.� Remove the
> > > >> ����cpu_unset variable.
> > > >>
> > ����(check_effective_target_arm_v8_3a_fp16_complex_neon_ok_nocache):
> > > >> ����Likewise.
> > > >> ---
> > > >> � gcc/testsuite/lib/target-supports.exp | 31
> > +++++++++++++++++----------
> > > >> � 1 file changed, 20 insertions(+), 11 deletions(-)
> > > >>
> > > >> diff --git a/gcc/testsuite/lib/target-supports.exp
> > b/gcc/testsuite/lib/target-supports.exp
> > > >> index dbcba42629f..210e246c955 100644
> > > >> --- a/gcc/testsuite/lib/target-supports.exp
> > > >> +++ b/gcc/testsuite/lib/target-supports.exp
> > > >> @@ -13717,27 +13717,32 @@ proc check_effective_target_inff { } {
> > > >> � proc check_effective_target_arm_v8_3a_complex_neon_ok_nocache {
> > } {
> > > >> ����� global et_arm_v8_3a_complex_neon_flags
> > > >> ����� set et_arm_v8_3a_complex_neon_flags ""
> > > >> -��� set cpu_unset ""
> > > >> � ����� if { ![istarget arm*-*-*] && ![istarget aarch64*-*-*] } {
> > > >> ����� return 0;
> > > >> ����� }
> > > >> � ����� if { [istarget arm*-*-*] } {
> > > >> -��� set cpu_unset "-mcpu=unset"
> > > >> +��� set flag_opts {
> > >
> > > We should try "" first, before attempting to modify the architecture.
> >
> > That will defeat the purpose of this patch, since you need to be able to
> > "reset" -mfpu by setting it to "auto", much like you need to do
> > "-mcpu=unset" in every alternative here. So I'll prepend "-mcpu=unset
> > -mfpu=auto" instead of "" to the list and resend as v2, would that be OK
> > with you?
> >
>
> No. The first pass with "" checks to see if the /default/ flags (from the
> compiler configuration or the testsuite options) are already suitable for
> running the test. We only use it if the test (below) then passes. We only
> try to override those flags if the defaults are /not/ sufficient; and we only
> need -mcpu=unset when setting the architecture (to ensure it doesn't conflict
> with any existing defaults).
If you look at the way
check_effective_target_arm_v8_3a_complex_neon_ok_nocache does
check-compile now:
foreach flags {"" "-mfloat-abi=softfp -mfpu=auto" "-mfloat-abi=hard
-mfpu=auto"} {
if { [check_no_compiler_messages_nocache \
arm_v8_3a_complex_neon_ok assembly {
#if !defined (__ARM_FEATURE_COMPLEX)
#error "__ARM_FEATURE_COMPLEX not defined"
#endif
#include <complex.h>
} "$flags $cpu_unset -march=armv8.3-a+simd"] } {
set et_arm_v8_3a_complex_neon_flags "$flags $cpu_unset
-march=armv8.3-a+simd"
return 1;
}
}
you'll see that even though $flags is "" on the first iteration of the
foreach loop, the actual compilation is performed with -mcpu=unset
-march=armv8.3-a+simd. Since the patch moves all flags to the $flags
variable, the base alternative has to have at least -mcpu=unset
-march=armv8.3-a+simd to preserve the current behavior.
Thus, I think the options for arm should start with either "-mcpu=unset
-march=armv8.3-a" (full compatibility with existing behavior) or
"-mcpu=unset" (shorter and also looks correct). For
check_effective_target_arm_v8_3a_complex_neon_ok_nocache, an additional
"-mfpu=auto" is required for all alternatives to negate the
"-mfpu=neon-fp16" inserted by advsimd-intrinsics.exp.
Hope this clarifies things,
Artemiy
>
>
>
> > >
> > > >> +������� "-mcpu=unset -mfpu=auto -march=armv8.3-a+simd"
> > > >> +������� "-mcpu=unset -mfloat-abi=softfp -mfpu=auto
> > -march=armv8.3-a+simd"
> > > >> +������� "-mcpu=unset -mfloat-abi=hard -mfpu=auto
> > -march=armv8.3-a+simd"
> > > >> +��� }
> > > >> +��� } else {
> > > >> +��� set flag_opts { "" "-march=armv8.3-a" }
> > > >> ����� }
> > > >
> > > > While this had no visible effect with the configurations I/we have
> > tested, I suspect we still want to start with just "-mcpu=unset" on arm.
> > Richard?
> > >
> > > We only want to add -mcpu=unset if we're overriding the defaults.
> > And since we should try "" before trying anything else, it has to be part
> > of the options where this is needed.
> > >
> >
> > Yes, that's what I meant.
> >
> >
> > > AFAIK aarch64 doesn't support -mcpu=unset (yet), so it's correct not
> > to add that for the aarch64 list.
> >
> > Yes, I didn't mean to add it to aarch64.
> >
> > >
> > > >
> > > > And on aarch64, dropping +simd seems harmless at the moment, but
> > maybe we want to keep it in order to be more future-proof?
> > >
> > > I'll let Tamar make the call on that one. But as things stand we
> > always default to having simd on aarch64 and I expect there will be a huge
> > amount of testsuite churn if that ever changes, so I'm not too worried
> > about that.
> >
> > Yeah, every base arch on aarch64 has +simd as per aarch64-arches.def:33,
> > so armv8.3-a here should be enough.
> >
> > Thanks,
> > Artemiy
> >
> > Given the patch removes +simd in the aarch64 without mentioning it in the
> > commit message, it will not be clear in the future if this was changed on
> > purpose or not.
> >
> >
>
