Patchwork scalar vector shift expansion problem on 64-bit

login
register
mail settings
Submitter Jakub Jelinek
Date Oct. 28, 2011, 6:41 p.m.
Message ID <20111028184130.GV1052@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/122479/
State New
Headers show

Comments

Jakub Jelinek - Oct. 28, 2011, 6:41 p.m.
On Fri, Oct 28, 2011 at 06:50:49PM +0200, Jakub Jelinek wrote:
> On Fri, Oct 28, 2011 at 09:07:31AM -0700, Richard Henderson wrote:
> > I think this is the same problem as Jakub is attacking here:
> > 
> >   http://gcc.gnu.org/ml/gcc-patches/2011-10/msg02503.html
> 
> It has been checked in already.  But my patch only deals
> with the vector << vector case, vector << scalar (including
> vector << scalar implemented using vector << vector) supposedly still
> needs fold_const somewhere if the type sizes disagree.

A wild guess, though untested, because I don't have a reproducer:

2011-10-28  Jakub Jelinek  <jakub@redhat.com>

	* tree-vect-stmts.c (vectorizable_shift): If op1 is vect_external_def
	and has different type from op0, cast it to op0's type before the
	loop first.



	Jakub
Richard Henderson - Oct. 28, 2011, 6:44 p.m.
On 10/28/2011 11:41 AM, Jakub Jelinek wrote:
> On Fri, Oct 28, 2011 at 06:50:49PM +0200, Jakub Jelinek wrote:
>> On Fri, Oct 28, 2011 at 09:07:31AM -0700, Richard Henderson wrote:
>>> I think this is the same problem as Jakub is attacking here:
>>>
>>>   http://gcc.gnu.org/ml/gcc-patches/2011-10/msg02503.html
>>
>> It has been checked in already.  But my patch only deals
>> with the vector << vector case, vector << scalar (including
>> vector << scalar implemented using vector << vector) supposedly still
>> needs fold_const somewhere if the type sizes disagree.
> 
> A wild guess, though untested, because I don't have a reproducer:
> 
> 2011-10-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* tree-vect-stmts.c (vectorizable_shift): If op1 is vect_external_def
> 	and has different type from op0, cast it to op0's type before the
> 	loop first.

I suspect the problem is in optabs.c, not here.

I'll try to look at it later today.


r~
Jakub Jelinek - Oct. 28, 2011, 8:55 p.m.
On Fri, Oct 28, 2011 at 11:44:17AM -0700, Richard Henderson wrote:
> > A wild guess, though untested, because I don't have a reproducer:
> > 
> > 2011-10-28  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	* tree-vect-stmts.c (vectorizable_shift): If op1 is vect_external_def
> > 	and has different type from op0, cast it to op0's type before the
> > 	loop first.
> 
> I suspect the problem is in optabs.c, not here.

Possible.  Though, if I disable all "ashl<mode>3", "lshr<mode>3" and
"ashr<mode>3" expanders in sse.md, I get without the above patch ICEs on
e.g.

long long d[64], e, j[64];

void
f4 (void)
{
  int i;
  for (i = 0; i < 64; i++)
    j[i] = d[i] << e;
}

with -O3 -mxop and -O3 -mavx2 and the patch fixes those.

	Jakub

Patch

--- gcc/tree-vect-stmts.c.jj	2011-10-28 16:21:06.000000000 +0200
+++ gcc/tree-vect-stmts.c	2011-10-28 20:19:27.000000000 +0200
@@ -2483,6 +2483,13 @@  vectorizable_shift (gimple stmt, gimple_
                  dealing with vectors of short/char.  */
               if (dt[1] == vect_constant_def)
                 op1 = fold_convert (TREE_TYPE (vectype), op1);
+	      else if (!useless_type_conversion_p (TREE_TYPE (vectype),
+						   TREE_TYPE (op1)))
+		{
+		  op1 = fold_convert (TREE_TYPE (vectype), op1);
+		  op1 = vect_init_vector (stmt, op1, TREE_TYPE (vectype),
+					  NULL);
+		}
             }
         }
     }