Re: [PATCH] fix PR middle-end/22480

[Dorit Naishlos <DORIT at il dot ibm dot com>]

> Hello!
>
>   This patch fixes the problem with vector shifts, where shift value
> is defined as a scalar value. This upsets vectoriser, which expects
> that vectors are shifted by a vector of values.
>
>   The proposed solution is, that vectorizer looks into insn pattern
> that corresponds to vectorizer optab, and checks the mode of operand 2
> (shift value in case of vectoriser shifts). If the mode of operand 2
> is scalar mode, then simply use a scalar or loop invariant shift value
> as paramtere to the vector operation optab.
>

Don't you want to make sure that op1 is actually constant/invariant?
You can do that calling
'vect_is_simple_use (op1, loop_vinfo, &def_stmt1, &def1, &dt1);'
and you want to check that
dt1 == vect_invariant_def || dt1 == vect_invariant_def.
(by the way, vect_is_simple_use is already called earlier in that function,
and the last value of 'dt' should hold the information you need).

Also, do we really want to allow this for any vector operation, or only for
vector shifts? (are there any other vector operations that behave like
this?)

Maybe we also want to add a comment about shr/shl_optab and/or about
LSHIFT/RSHIFT_EXPR, explaining the behavior when the second argument is a
scalar (because the default interpretation of vector operations of the form
'v3=op<v1,v2>' in which 'op' is also defined for scalar types, is that it
is equivalent to:
      for (i=0;i<num_elements_in_vector;i++)
            v3[i] = op<v1[i],v2[i])
so maybe we should define the behavior when the second operand is not a
vector).

dorit


>   The testcase:
>
> void
> test_1 (void)
> {
>   static unsigned int bm[16];
>   int j;
>   for (j = 0; j < 16; j++)
>     bm[j] <<= 8;
> }
>
> void
> test_2 (int a)
> {
>   static unsigned int bm[16];
>   int j;
>   for (j = 0; j < 16; j++)
>     bm[j] <<= a;
> }
>
> then compiles to:
>
> (vector shift by constant)
> test_1:
> pushl %ebp
> movl $bm.1591, %eax
> movl %esp, %ebp
> movl $bm.1591, %edx
> .p2align 4,,15
> .L2:
> movdqa (%eax), %xmm0
> addl $4, %edx
> pslld $8, %xmm0
> movdqa %xmm0, (%eax)
> addl $16, %eax
> cmpl $bm.1591+16, %edx
> jne .L2
> popl %ebp
> ret
>
> and
>
> (vector shift by parameter)
> test_2:
> pushl %ebp
> movl $bm.1602, %eax
> movl %esp, %ebp
> movl $bm.1602, %edx
> movd 8(%ebp), %xmm1
> .p2align 4,,15
> .L9:
> movdqa (%eax), %xmm0
> addl $4, %edx
> pslld %xmm1, %xmm0
> movdqa %xmm0, (%eax)
> addl $16, %eax
> cmpl $bm.1602+16, %edx
> jne .L9
> popl %ebp
> ret
>
> 2005-09-07  Uros Bizjak  <uros@kss-loka.si>
>
> PR middle-end/22480
> * tree-vect-transform.c (vectorizable_operation): Fallback to
> scalar mode for tree operand 1, if operand 2 of vector insn pattern
> can not handle vector modes.
> * Makefile.in (tree-vect-transform.o): Depend on recog.h.
>
>
> testsuite/ChangeLog:
>
> 2005-09-07  Uros Bizjak  <uros@kss-loka.si>
>
> PR middle-end/22480
> * gcc.dg/vect/pr22480.c: New test.
>
> Patch was regtested for c anc c++ on i686-pc-linux-gnu.
>
> Uros.
> [attachment "pr22480.diff" deleted by Dorit Naishlos/Haifa/IBM]
> [attachment "pr22480.c" deleted by Dorit Naishlos/Haifa/IBM]