diff mbox

[PR52252] Vectorization for load/store groups of size 3.

Message ID CAOvf_xx0dcnzBP89sGgGeKZe8=xBFnmy1C-jJTd7COKRVDC0tQ@mail.gmail.com
State New
Headers show

Commit Message

Evgeny Stupachenko May 16, 2014, 1:03 p.m. UTC
The test uses SSSE3 because of the following restriction in i386.c:

static bool
expand_vec_perm_pshufb2 (struct expand_vec_perm_d *d)
{
  rtx rperm[2][16], vperm, l, h, op, m128;
  unsigned int i, nelt, eltsz;

  if (!TARGET_SSSE3 || GET_MODE_SIZE (d->vmode) != 16)
    return false;

Does the following fix ok?

2014-05-16  Evgeny Stupachenko  <evstupac@gmail.com>

       * gcc.dg/vect/pr52252-ld.c: Fix target for the test.


On Tue, May 13, 2014 at 12:21 PM, Richard Biener <rguenther@suse.de> wrote:
> On Mon, 12 May 2014, Evgeny Stupachenko wrote:
>
>> The test is on general changes. However I was able to test it on x86 only.
>> I see 2 possible solutions:
>> 1. Set the test for x86 only.
>> 2. Modify it so that it will pass on sparc-sun-solaris2.
>>
>> If 2. is not acceptable I'll create patch for 1.
>> Currently I don't see why "in0_9 = *in_27" is not supported. Does the
>> test fail because of unsupported permutation?
>
> The test uses
>
> /* { dg-options "-O2 -g -ftree-vectorize -mssse3 -fdump-tree-vect-details"
> { target { i?86-*-* x86_64-*-* } } } */
>
> that's bogus.  You shouldn't add any dg-options.  Instead use proper
> dg-effective-target checks for the mssse3 feature you are using.
> Note that the dg-final checking is applied regardless of the options
> above are applied or not.
>
> Why does the test only succeed with -mssse3 btw?
>
> The proper way to restrict the test to a single target is to use
>
> /* { dg-skip-if "why" { ! { x86_64-*-* i?86-*-* } } } */
>
> Sorry for not catching this in the review.
>
> Richard.
>
>>
>> On Mon, May 12, 2014 at 7:14 PM, Rainer Orth
>> <ro@cebitec.uni-bielefeld.de> wrote:
>> > Evgeny Stupachenko <evstupac@gmail.com> writes:
>> >
>> >> Patch with fixes attached.
>> >> Currently if-structure is as following:
>> >> +      if (count == 3)
>> >> ...
>> >> +      else
>> >> +       {
>> >> +         /* If length is not equal to 3 then only power of 2 is supported.  */
>> >> +         gcc_assert (exact_log2 (count) != -1);
>> >>
>> >> For stores group I've created another mail thread.
>> > [...]
>> >>>> 2014-05-06  Evgeny Stupachenko  <evstupac@gmail.com>
>> >>>>
>> >>>>        PR tree-optimization/52252
>> >>>>        * gcc.dg/vect/pr52252-ld.c: Test on loads group of size 3.
>> >
>> > This test FAILs on sparc-sun-solaris2.11, both 32 and 64-bit:
>> >
>> > FAIL: gcc.dg/vect/pr52252-ld.c scan-tree-dump-times vect "vectorized 1 loops" 1
>> > FAIL: gcc.dg/vect/pr52252-ld.c -flto -ffat-lto-objects  scan-tree-dump-times vect "vectorized 1 loops" 1
>> >
>> > The dumps have
>> >
>> > /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: not vectorized: relevant stmt not supported: in0_9 = *in_27;
>> > /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/vect/pr52252-ld.c:7:1: note: vectorized 0 loops in function.
>> >
>> >         Rainer
>> >
>> > --
>> > -----------------------------------------------------------------------------
>> > Rainer Orth, Center for Biotechnology, Bielefeld University
>>
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

Comments

Rainer Orth May 16, 2014, 1:11 p.m. UTC | #1
Hi Evgeny,

> Does the following fix ok?
>
> 2014-05-16  Evgeny Stupachenko  <evstupac@gmail.com>
>
>        * gcc.dg/vect/pr52252-ld.c: Fix target for the test.
>
>
> diff --git a/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
> b/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
> index 6e3cb52..301433b 100644
> --- a/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
> +++ b/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -g -ftree-vectorize -mssse3
> -fdump-tree-vect-details" { target { i?86-*-* x86_64-*-* } } } */
> +/* { dg-options "-O2 -g -ftree-vectorize -mssse3 -fdump-tree-vect-details" } */
> +/* { dg-skip-if "why" { ! { x86_64-*-* i?86-*-* } } } */

If the test is really x86 specific, move it to gcc.target/i386 and
remove the dg-skip-if.  Otherwise, add an explanation for skipping the
test on other targets to the first arg of dg-skip-if.  This is supposed
to be a comment stating why the test is skipped, not "why" literally.

Approval or rejection of the testcase is up to the target maintainers.

	Rainer
Jakub Jelinek May 16, 2014, 1:21 p.m. UTC | #2
On Fri, May 16, 2014 at 03:11:05PM +0200, Rainer Orth wrote:
> Hi Evgeny,
> 
> > Does the following fix ok?
> >
> > 2014-05-16  Evgeny Stupachenko  <evstupac@gmail.com>
> >
> >        * gcc.dg/vect/pr52252-ld.c: Fix target for the test.
> >
> >
> > diff --git a/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
> > b/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
> > index 6e3cb52..301433b 100644
> > --- a/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
> > +++ b/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
> > @@ -1,5 +1,6 @@
> >  /* { dg-do compile } */
> > -/* { dg-options "-O2 -g -ftree-vectorize -mssse3
> > -fdump-tree-vect-details" { target { i?86-*-* x86_64-*-* } } } */
> > +/* { dg-options "-O2 -g -ftree-vectorize -mssse3 -fdump-tree-vect-details" } */
> > +/* { dg-skip-if "why" { ! { x86_64-*-* i?86-*-* } } } */
> 
> If the test is really x86 specific, move it to gcc.target/i386 and
> remove the dg-skip-if.  Otherwise, add an explanation for skipping the
> test on other targets to the first arg of dg-skip-if.  This is supposed
> to be a comment stating why the test is skipped, not "why" literally.

Well, I don't see anything i?86/x86_64 specific on the test.  What
is specific is the -mssse3, which supposedly should be added through
/* { dg-additional-options "-mssse3" { target { i?86-*-* x86_64-*-* } } } */
and then perhaps the test might not necessarily be vectorized (so the
dg-final line may need target guard as well.
But, I see no reason not to try to compile this on other targets.

	Jakub
Evgeny Stupachenko May 16, 2014, 1:33 p.m. UTC | #3
I'm not sure about other architectures. I can test it on x86. Most
likely it will pass on Arm, but again I'm not sure that { arm*-*-* }
is ok.

The test is on general changes. So we can wait for others and if there
are no more objections leave sparc-sun-solaris2 as target to skip.

Or change to the following:
/* { dg-skip-if "The test should pass on x86, other architectures are
untested" { ! { x86_64-*-* i?86-*-* } } } */

So that other will add their targets if necessary.

Thanks,
Evgeny
Rainer Orth May 16, 2014, 1:38 p.m. UTC | #4
Evgeny Stupachenko <evstupac@gmail.com> writes:

> The test is on general changes. So we can wait for others and if there
> are no more objections leave sparc-sun-solaris2 as target to skip.

If so, use sparc*-*-* instead.

> Or change to the following:
> /* { dg-skip-if "The test should pass on x86, other architectures are
> untested" { ! { x86_64-*-* i?86-*-* } } } */
>
> So that other will add their targets if necessary.

I wouldn't do it this way, because this will never happen.  Rather,
start with all targets and skip or xfail if necessary, adding an
explanation why this is necessary.

	Rainer
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
b/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
index 6e3cb52..301433b 100644
--- a/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
+++ b/gcc/testsuite/gcc.dg/vect/pr52252-ld.c
@@ -1,5 +1,6 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -g -ftree-vectorize -mssse3
-fdump-tree-vect-details" { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-options "-O2 -g -ftree-vectorize -mssse3 -fdump-tree-vect-details" } */
+/* { dg-skip-if "why" { ! { x86_64-*-* i?86-*-* } } } */

 #define byte unsigned char