Patchwork [rs6000] Fix rs6000_expand_vector_set for little endian

login
register
mail settings
Submitter William J. Schmidt
Date Nov. 1, 2013, 1:45 a.m.
Message ID <1383270347.6275.241.camel@gnopaine>
Download mbox | patch
Permalink /patch/287690/
State New
Headers show

Comments

William J. Schmidt - Nov. 1, 2013, 1:45 a.m.
Hi,

Brooks Moses reported a bug with code that sets a single element of a
vector to a given value and the rest of the vector to zero.  This is
implemented in rs6000_expand_vector_set, which uses a vperm instruction
to place the nonzero value.  As usual, we need to adjust the permute
control vector and swap the order of the input operands.  I added a test
case based on the bug report.

Bootstrapped and tested on powerpc64{,le}-unknown-linux-gnu with no
regressions.  The new test now passes for both endiannesses.  Is this ok
for trunk?

Thanks,
Bill


gcc:

2013-10-31  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>

	* config/rs6000/rs6000.c (rs6000_expand_vector_set): Adjust for
	little endian.

gcc/testsuite:

2013-10-31  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>

	* gcc.dg/vmx/vec-set.c: New.
David Edelsohn - Nov. 2, 2013, 3:14 p.m.
On Thu, Oct 31, 2013 at 9:45 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> Brooks Moses reported a bug with code that sets a single element of a
> vector to a given value and the rest of the vector to zero.  This is
> implemented in rs6000_expand_vector_set, which uses a vperm instruction
> to place the nonzero value.  As usual, we need to adjust the permute
> control vector and swap the order of the input operands.  I added a test
> case based on the bug report.
>
> Bootstrapped and tested on powerpc64{,le}-unknown-linux-gnu with no
> regressions.  The new test now passes for both endiannesses.  Is this ok
> for trunk?
>
> Thanks,
> Bill
>
>
> gcc:
>
> 2013-10-31  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
>
>         * config/rs6000/rs6000.c (rs6000_expand_vector_set): Adjust for
>         little endian.

This is okay.

> +  if (!BYTES_BIG_ENDIAN)

But negating the expression and reversing the branches seems superfluous.

Thanks, David

Patch

Index: gcc/testsuite/gcc.dg/vmx/vec-set.c
===================================================================
--- gcc/testsuite/gcc.dg/vmx/vec-set.c	(revision 0)
+++ gcc/testsuite/gcc.dg/vmx/vec-set.c	(revision 0)
@@ -0,0 +1,14 @@ 
+#include "harness.h"
+
+vector short
+vec_set (short m)
+{
+  return (vector short){m, 0, 0, 0, 0, 0, 0, 0};
+}
+
+static void test()
+{
+  check (vec_all_eq (vec_set (7),
+		     ((vector short){7, 0, 0, 0, 0, 0, 0, 0})),
+	 "vec_set");
+}
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 204192)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -5529,10 +5529,27 @@  rs6000_expand_vector_set (rtx target, rtx val, int
     XVECEXP (mask, 0, elt*width + i)
       = GEN_INT (i + 0x10);
   x = gen_rtx_CONST_VECTOR (V16QImode, XVEC (mask, 0));
-  x = gen_rtx_UNSPEC (mode,
-		      gen_rtvec (3, target, reg,
-				 force_reg (V16QImode, x)),
-		      UNSPEC_VPERM);
+
+  if (!BYTES_BIG_ENDIAN)
+    {
+      /* Invert selector.  */
+      rtx splat = gen_rtx_VEC_DUPLICATE (V16QImode,
+					 gen_rtx_CONST_INT (QImode, -1));
+      rtx tmp = gen_reg_rtx (V16QImode);
+      emit_move_insn (tmp, splat);
+      x = gen_rtx_MINUS (V16QImode, tmp, force_reg (V16QImode, x));
+      emit_move_insn (tmp, x);
+
+      /* Permute with operands reversed and adjusted selector.  */
+      x = gen_rtx_UNSPEC (mode, gen_rtvec (3, reg, target, tmp),
+			  UNSPEC_VPERM);
+    }
+  else 
+    x = gen_rtx_UNSPEC (mode,
+			gen_rtvec (3, target, reg,
+				   force_reg (V16QImode, x)),
+			UNSPEC_VPERM);
+
   emit_insn (gen_rtx_SET (VOIDmode, target, x));
 }