Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> writes:
> On Thu, 30 May 2019 at 21:19, Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>>
>> Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> writes:
>> > On Thu, 30 May 2019 at 15:10, Richard Sandiford
>> > <richard.sandif...@arm.com> wrote:
>> >>
>> >> Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> 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.
> 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 [...]).
Thanks,
Richard
