[AArch64] [SVE] PR88837 - Poor vector construction code in VL-specific mode
Prathamesh Kulkarni
prathamesh.kulkarni@linaro.org
Mon Jun 3 09:52:00 GMT 2019
On Mon, 3 Jun 2019 at 14:53, Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > On Thu, 30 May 2019 at 21:19, Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> >>
> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> >> > On Thu, 30 May 2019 at 15:10, Richard Sandiford
> >> > <richard.sandiford@arm.com> wrote:
> >> >>
> >> >> Prathamesh Kulkarni <prathamesh.kulkarni@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.
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: target-supports-sve-2.diff
Type: text/x-patch
Size: 1234 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20190603/7d64cf7b/attachment.bin>
More information about the Gcc-patches
mailing list