This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Quoting Paolo Bonzini <paolo.bonzini@lu.unisi.ch>:

> >You need to add something along these lines before the tranform part in
> >vectorizable_operation:
> >
> >  if (code == LSHIFT_EXPR || code == RSHIFT_EXPR)
> >    {
> >      optab_op2_mode = insn_data[icode].operand[2].mode;
> >      if (VECTOR_MODE_P (vec_mode)
> >  
> >
> this is also wrong.  when using generic vectorization, an invariant 
> argument is required because you cannot vectorize a loop like

This check was introduced because generic vectorization can produce an
scalar integer operand (that equals char[4] in a testcase below). However, 
scalar shift insn pattern is not able to shift separate char elements of an 
integer.

> void convert_32_to_8(void)
> {
>   static unsigned char bm[16];
>   char am[16];
>   int j;
>   for (j=0; j<16;j++)
>     bm[j]<<=am[j];
> }

The problem is, that produced integer in above case can not be shifted by 
{op1, op1, op1, op1} - even if op1 is loop invariant without many logic 
operations (as shown below), but SSE pattern can shift separate elements in the 
vector by the same shift value.

To solve this, I would like to introduce this transformation: when elements of 
a vector are shifted by an array of values, this operation could be transformed 
to the shift of a vector by a scalar, when the value of shift is loop 
invariant. The shift of a vector by a scalar is the shift operation that is 
modelled by SSE insn patterns.

To handle the example above, I would suggest to skip this shift transformation, 
when vec_mode is not !VECTOR_MODE_P (vec_mode), that is, when we use scalar 
insn pattern to emulate vector operation.

Following all recent suggestions, I would like to propose following code:

  if (op_type == binary_op)
    {
      op1 = TREE_OPERAND (operation, 1);

      if (code == LSHIFT_EXPR || code == RSHIFT_EXPR)
	{
	  /* Vector shl and shr insn patterns can be defined with
	     scalar operand 2 (shift operand).  In this case, use
	     constant or loop invariant op1 directly, without
	     extending it to vector mode first.  */

	  /* FORNOW: not yet supported.  */
	  if (!VECTOR_MODE_P (vec_mode))
	    return false;

	  optab_op2_mode = insn_data[icode].operand[2].mode;
	  if (!VECTOR_MODE_P (optab_op2_mode))
	    {
	      if ((dt == vect_constant_def
		   || dt == vect_invariant_def))
		{
		  if (vect_print_dump_info (REPORT_DETAILS))
		    fprintf (vect_dump, "operand 1 using scalar mode.");
		  vec_oprnd1 = op1;
		}
	      else
		{
		  if (vect_print_dump_info (REPORT_DETAILS))
		    fprintf (vect_dump, "operand mode requires"
			     " invariant argument.");
		  return false;
		}
	    }
	}

In the code above, we skip pseudo-vectorised operations (when the operation 
itself is in fact scalar operation), we also skip transformation for non-
invariant arguments.

However, there are some problems. For Dorit's example with non-invariant 
arguments, the code above returns false, but in vect_transform_stmt() function, 
we further trigger an assert:

  ...
  if (STMT_VINFO_RELEVANT_P (stmt_info))
    {
      switch (STMT_VINFO_TYPE (stmt_info))
      {
      case op_vec_info_type:
	done = vectorizable_operation (stmt, bsi, &vec_stmt);
	gcc_assert (done);
	break;
  ...

I'm a little lost here, but it seems to me that returning false from 
vectorizable_operation() isn't the right thing to do to skip the transform.

Uros.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]