On 19/12/2024 12:17, Christophe Lyon wrote:
> The vect testsuite adds -mfpu=neon before the arm_v8_3a_complex_neon
> flags via check_vect_support_and_set_flags, so before this change
> testcases are compiled with -mfpu=neon (and no -march/-mfloat-abi
> flag) with an arm-linux-gnueabihf toolchain configured using
> --with-float=hard --with-fpu=vfpv3-d16 --with-arch=armv7-a
>
> However, when computing et_arm_v8_3a_complex_neon_flags, -mfpu=neon is
> not used, and the first attempt with flags="" uses
> -mcpu=unset -march=armv8.3-a, resulting in
> error: #error "__ARM_FEATURE_COMPLEX not defined"
>
> The same occurs with -mfloat-abi=softfp -mfpu=auto -mcpu=unset
> -march=armv8.3-a,
> but -mfloat-abi=hard -mfpu=auto -mcpu=unset -march=armv8.3-a fails with:
> error: '-mfloat-abi=hard': selected architecture lacks an FPU
>
> So finally et_arm_v8_3a_complex_neon_flags is empty and
> check_effective_target_arm_v8_3a_complex_neon_ok_nocache returns 0,
> leading to the behavior described in the first paragraph.
>
> Since -march=armv8.3-a alone does not enable any FPU, it looks like an
> oversight and +simd should be added (this also gives some sense to the
> -mfpu=auto option which is used along -mfloat-abi=XXX, and to _neon_
> in the effective-target name).
>
> Adding +simd thus means that trying with flags="" will still fail (now
> using -mcpu=unset -march=armv8.3-a+simd):
> error: #error "__ARM_FEATURE_COMPLEX not defined"
>
> but -mfloat-abi=softfp -mfpu=auto -mcpu=unset -march=armv8.3-a+simd
> succeeds and testcases are now compiled with
> -mfpu=neon -mfloat-abi=softfp -mfpu=auto -mcpu=unset -march=armv8.3-a+simd
> leading to compilation errors when they #include <complex.h> or <stdint.h>:
> error: gnu/stubs-soft.h: No such file or directory
>
> Adding #include <stdint.h> to the sample code for the
> effective-targets solves this problem and we now compile the tests
> with
> -mfpu=neon -mfloat-abi=hard -mfpu=auto -mcpu=unset -march=armv8.3-a+simd
>
> This patch does not enable more tests, but now we have
> FAIL: gcc.dg/vect/complex/fast-math-complex-mls-float.c scan-tree-dump vect
> "Found COMPLEX_ADD_ROT270"
> which used to pass with -mfpu=neon, instead of the now implied
> neon-fp-armv8, but that looks like a bug?
>
> gcc/testsuite/ChangeLog:
>
> * lib/target-supports.exp
> (check_effective_target_arm_v8_3a_complex_neon_ok_nocache): Add
> +simd, include stdint.h.
> ---
> gcc/testsuite/lib/target-supports.exp | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/lib/target-supports.exp
> b/gcc/testsuite/lib/target-supports.exp
> index 9f4e2700dd2..59eb38344ed 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -13256,8 +13256,9 @@ proc
> check_effective_target_arm_v8_3a_complex_neon_ok_nocache { } {
> #if !defined (__ARM_FEATURE_COMPLEX)
> #error "__ARM_FEATURE_COMPLEX not defined"
> #endif
> - } "$flags -mcpu=unset -march=armv8.3-a"] } {
> - set et_arm_v8_3a_complex_neon_flags "$flags -mcpu=unset
> -march=armv8.3-a"
> + #include <stdint.h>
> + } "$flags -mcpu=unset -march=armv8.3-a+simd"] } {
> + set et_arm_v8_3a_complex_neon_flags "$flags -mcpu=unset
> -march=armv8.3-a+simd"
> return 1;
> }
> }
So there are two parts to this. Firstly, adding +simd, which is fine.
The second is adding the check that stdint.h can be read, which needs some
careful consideration. The need to be able to parse stdint.h comes from the
desire to include "arm_neon.h", which is slightly orthogonal to the need to
enable simd instructions as part of the architecture. It's a legitimate thing
to need to test for, but I'd like it to be reflected in the effective-target
test name so that tests that don't need arm_neon.h need not be affected.
As far as I can tell, this specific target check is only used in two tests and
both of these also include arm_neon.h. So I think in this case we can achieve
both goals by renaming this test to make that more explicit, something like
arm_v8_3a_complex__arm_neon_h_ok, for example? Whichever scheme we use should
probably be documented (in a generic way) in doc/sourcebuild.texi as well.
R.
