Patchwork [PowerPC] altivec_expand_vec_perm_const reverses pack pattern arguments in little endian mode

login
register
mail settings
Submitter William J. Schmidt
Date July 22, 2013, 11:05 p.m.
Message ID <1374534309.3633.96.camel@gnopaine>
Download mbox | patch
Permalink /patch/260833/
State New
Headers show

Comments

William J. Schmidt - July 22, 2013, 11:05 p.m.
This patch is another fix for vector handling in little endian mode.
The first two operands for a pack pattern are two vector registers that
form a contiguous array of inputs.  In LE mode the order of the operands
must be reversed so that the array remains contiguous in the reverse
order.

This fixes a failure in the testsuite when run little-endian
(gcc.dg/vect/no-scevccp-outer-18.c).  Bootstrapped and tested big-endian
on powerpc64-unknown-linux-gnu with no new regressions.  Ok for trunk?

Patch by Anton Blanchard.

Thanks,
Bill


2013-07-22  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Anton Blanchard <anton@au1.ibm.com>

	* config/rs6000/rs6000.c (altivec_expand_vec_perm_const): Reverse
	two operands for little-endian.
David Edelsohn - July 22, 2013, 11:49 p.m.
On Mon, Jul 22, 2013 at 7:05 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> This patch is another fix for vector handling in little endian mode.
> The first two operands for a pack pattern are two vector registers that
> form a contiguous array of inputs.  In LE mode the order of the operands
> must be reversed so that the array remains contiguous in the reverse
> order.
>
> This fixes a failure in the testsuite when run little-endian
> (gcc.dg/vect/no-scevccp-outer-18.c).  Bootstrapped and tested big-endian
> on powerpc64-unknown-linux-gnu with no new regressions.  Ok for trunk?
>
> Patch by Anton Blanchard.
>
> Thanks,
> Bill
>
>
> 2013-07-22  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>             Anton Blanchard <anton@au1.ibm.com>
>
>         * config/rs6000/rs6000.c (altivec_expand_vec_perm_const): Reverse
>         two operands for little-endian.

Wouldn't it be better to handle this where the code is performing a
swap a few lines above?

Thanks, David
William J. Schmidt - July 23, 2013, 12:56 a.m.
On Mon, 2013-07-22 at 19:49 -0400, David Edelsohn wrote:
> On Mon, Jul 22, 2013 at 7:05 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > This patch is another fix for vector handling in little endian mode.
> > The first two operands for a pack pattern are two vector registers that
> > form a contiguous array of inputs.  In LE mode the order of the operands
> > must be reversed so that the array remains contiguous in the reverse
> > order.
> >
> > This fixes a failure in the testsuite when run little-endian
> > (gcc.dg/vect/no-scevccp-outer-18.c).  Bootstrapped and tested big-endian
> > on powerpc64-unknown-linux-gnu with no new regressions.  Ok for trunk?
> >
> > Patch by Anton Blanchard.
> >
> > Thanks,
> > Bill
> >
> >
> > 2013-07-22  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >             Anton Blanchard <anton@au1.ibm.com>
> >
> >         * config/rs6000/rs6000.c (altivec_expand_vec_perm_const): Reverse
> >         two operands for little-endian.
> 
> Wouldn't it be better to handle this where the code is performing a
> swap a few lines above?

Hm, I don't think so.  The reason for that swap has nothing to do with
endianness, so I think it would just confuse matters.  Also there's a
three-way swap going on with x, op0, and op1 there, and we just want to
swap op0 and op1.  I think the patch as it stands is probably easier to
grok.

Thanks,
Bill
> 
> Thanks, David
>
William J. Schmidt - July 23, 2013, 12:59 a.m.
On Mon, 2013-07-22 at 19:56 -0500, Bill Schmidt wrote:
> 
> On Mon, 2013-07-22 at 19:49 -0400, David Edelsohn wrote:
> > On Mon, Jul 22, 2013 at 7:05 PM, Bill Schmidt
> > <wschmidt@linux.vnet.ibm.com> wrote:
> > > This patch is another fix for vector handling in little endian mode.
> > > The first two operands for a pack pattern are two vector registers that
> > > form a contiguous array of inputs.  In LE mode the order of the operands
> > > must be reversed so that the array remains contiguous in the reverse
> > > order.
> > >
> > > This fixes a failure in the testsuite when run little-endian
> > > (gcc.dg/vect/no-scevccp-outer-18.c).  Bootstrapped and tested big-endian
> > > on powerpc64-unknown-linux-gnu with no new regressions.  Ok for trunk?
> > >
> > > Patch by Anton Blanchard.
> > >
> > > Thanks,
> > > Bill
> > >
> > >
> > > 2013-07-22  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> > >             Anton Blanchard <anton@au1.ibm.com>
> > >
> > >         * config/rs6000/rs6000.c (altivec_expand_vec_perm_const): Reverse
> > >         two operands for little-endian.
> > 
> > Wouldn't it be better to handle this where the code is performing a
> > swap a few lines above?
> 
> Hm, I don't think so.  The reason for that swap has nothing to do with
> endianness, so I think it would just confuse matters.  Also there's a
> three-way swap going on with x, op0, and op1 there, and we just want to
> swap op0 and op1.  I think the patch as it stands is probably easier to
> grok.

Bleah, sorry, wasn't paying enough attention.  Didn't notice x was being
reused as an intermediate there instead of its regular use.

It still seems a bit confusing to mix the two reasons for swapping, but
I'll look at it.

Thanks,
Bill
> 
> Thanks,
> Bill
> > 
> > Thanks, David
> >

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 201131)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -28526,7 +28529,12 @@  altivec_expand_vec_perm_const (rtx operands[4])
 	    x = target;
 	  else
 	    x = gen_reg_rtx (omode);
-	  emit_insn (GEN_FCN (icode) (x, op0, op1));
+	  /* For little-endian, the two input operands must be swapped
+	     to ensure proper right-to-left numbering from 0 to 2N-1.  */
+	  if (BYTES_BIG_ENDIAN)
+	    emit_insn (GEN_FCN (icode) (x, op0, op1));
+	  else
+	    emit_insn (GEN_FCN (icode) (x, op1, op0));
 	  if (omode != V16QImode)
 	    emit_move_insn (target, gen_lowpart (V16QImode, x));
 	  return true;