On Mon, 3 Jun 2019 at 14:53, Richard Sandiford wrote: > > Prathamesh Kulkarni writes: > > On Thu, 30 May 2019 at 21:19, Richard Sandiford > > wrote: > >> > >> Prathamesh Kulkarni writes: > >> > On Thu, 30 May 2019 at 15:10, Richard Sandiford > >> > wrote: > >> >> > >> >> Prathamesh Kulkarni writes: > >> >> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/init_1.c b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c > >> >> > new file mode 100644 > >> >> > index 00000000000..cbfeff4a59c > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c > >> >> > @@ -0,0 +1,27 @@ > >> >> > +/* { dg-do compile { target aarch64_asm_sve_ok } } */ > >> >> > +/* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256 --save-temps" } */ > >> >> > >> >> Sorry for not noticing last time, but the combination of aarch64_asm_sve_ok > >> >> and --save-temps only makes sense for assemble tests, not compile tests. > >> >> So these should either be: > >> >> > >> >> /* { dg-do assemble { target aarch64_asm_sve_ok } } */ > >> >> /* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256 --save-temps" } */ > >> >> > >> >> or: > >> >> > >> >> /* { dg-do compile } */ > >> >> /* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256" } */ > >> >> > >> >> Might as well as go for the first I guess. Same for the other > >> >> non-run tests. > >> >> > >> >> OK with that change, thanks. > >> > Thanks for pointing out, updated the patch with dg-do assemble. > >> > Sorry for silly ques - What configure option should be passed to gcc > >> > to generate code with -msve-vector-bits=256 by default ? > >> > I suppose that'd be necessary for correctness testing, to test patch > >> > with run tests that contain initializers and don't explicitly pass > >> > -msve-vector-bits=256 ? > >> > >> There's no configure option, but you can test with things like > >> --target_board unix/-msve-vector-bits=256 or > >> --target_board unix{,/-msve-vector-bits=256} (to test both with > >> and without -msve-vector-bits=256). > > Hi, > > Sorry for late response. I managed to cross-test using qemu and seeing some > > fallout: > > > > 1. Testing pristine trunk with --target_board=arm-qemu/-march=armv8.2-a+sve > > results in following ICE's: > > FAIL: gcc.c-torture/compile/pr82096.c -O0 (internal compiler error) > > FAIL: gcc.c-torture/compile/pr82096.c -O1 (internal compiler error) > > FAIL: gcc.c-torture/compile/pr82096.c -O2 (internal compiler error) > > FAIL: gcc.c-torture/compile/pr82096.c -O2 -flto > > -fno-use-linker-plugin -flto-partition=none (internal compiler error) > > FAIL: gcc.c-torture/compile/pr82096.c -O3 -g (internal compiler error) > > FAIL: gcc.c-torture/compile/pr82096.c -Os (internal compiler error) > > FAIL: gcc.dg/di-longlong64-sync-1.c (internal compiler error) > > FAIL: gcc.dg/di-sync-multithread.c (internal compiler error) > > FAIL: gcc.target/aarch64/pr87839.c (internal compiler error) > > > > 2. Passing -msve-vector-bits=256 results in following additional > > ICE's with trunk: > > FAIL: gcc.dg/pr88598-2.c (internal compiler error) > > FAIL: gcc.dg/pr88598-3.c (internal compiler error) > > FAIL: gcc.dg/pr88598-5.c (internal compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O1 (internal > > compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O2 (internal > > compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O2 -flto > > -fno-use-linker-plugin -flto-partition=none (internal compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O3 > > -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer > > -finline-functions (internal compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O3 -g > > (internal compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -Os (internal > > compiler error) > > FAIL: gcc.target/aarch64/pr87839.c (internal compiler error) > > FAIL: gfortran.dg/vect/vect-8-epilogue.F90 -O (internal compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O1 (internal > > compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O2 (internal > > compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O2 -flto > > -fno-use-linker-plugin -flto-partition=none (internal compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O3 > > -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer > > -finline-functions (internal compiler error) > > FAIL: c-c++-common/torture/builtin-convertvector-1.c -O3 -g > > (internal compiler error) > > I've been concentrating on the ACLE branch recently and haven't been > tracking trunk, so can well believe it. :-) > > > Applying patch doesn't results in additional fallout relative to trunk > > with -march=armv8.2-a+sve -msve-vector-bits=256. > > Is it OK to apply ? > > Yes, thanks. Thanks, committed as r271857. > > > PS: Initially, I got UNSUPPORTED for SVE tests, because assembler > > was rejecting the test "ptrue p0.b" in selector > > check_effective_target_aarch64_sve_hw and would accept it only if > > passed -march=armv8.2-a+sve explicitly on command line. I worked > > around that by patching lib/target-supports.exp to explicitly pass the > > option. Not sure if that's the right approach tho ? > > Ah, OK. I always test SVE with a toolchain configured for SVE by default > (to get extra coverage building the target libraries), so I never hit this. > For: > > > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > > index 3bd6e815715..0ff0d8fb757 100644 > > --- a/gcc/testsuite/lib/target-supports.exp > > +++ b/gcc/testsuite/lib/target-supports.exp > > @@ -3846,6 +3846,10 @@ proc add_options_for_arm_neon_softfp_fp16 { flags } { > > return "$flags $et_arm_neon_softfp_fp16_flags" > > } > > > > +proc add_options_for_arm_sve { flags } { > > + return "$flags -march=armv8.2-a+sve" > > ...this I think we should avoid overriding the flags if they already > select SVE, so probably: > > if { ![istarget aarch64*-*-*] || [check_effective_target_aarch64_sve] } { > return "$flags" > } > > Should be "aarch64_sve" rather than "arm_sve". > > > +} > > + > > # Return 1 if this is an ARM target supporting the FP16 alternative > > # format. Some multilibs may be incompatible with the options needed. Also > > # set et_arm_neon_fp16_flags to the best options to add. > > @@ -4323,7 +4327,7 @@ proc check_effective_target_aarch64_sve_hw { } { > > asm volatile ("ptrue p0.b"); > > return 0; > > } > > - }] > > + } [ add_options_for_arm_sve "" ]] > > } > > > > # Return true if this is an AArch64 target that can run SVE code and > > @@ -4343,7 +4347,7 @@ proc aarch64_sve_hw_bits { bits } { > > __builtin_abort (); > > return 0; > > } > > - }]] > > + }] [add_options_for_arm_sve ""] ] > > } > > > > # Return true if this is an AArch64 target that can run SVE code and > > Think the formatting in the second is preferred over the first (i.e. > no spaces inside the [...]). Does the attached patch look OK ? Thanks, Prathamesh > > Thanks, > Richard