Hi!
We had a problem report for code attempting to implement a vector right-shift
for a
vector long long (V2DImode) type. The programmer noted that we don't have a
vector
splat-immediate for this mode, but cleverly realized he could use a vector char
splat-immediate since only the lower 6 bits of the shift count are read. This
is a
documented feature of both the vector shift built-ins and the underlying
instruction.
Starting with GCC 8, the vector shifts are expanded early in
rs6000_gimple_fold_builtin.
However, the GIMPLE folding does not currently perform the required
TRUNC_MOD_EXPR to
implement the built-in semantics. It appears that this was caught earlier for
vector
shift-left and fixed, but the same problem was not fixed for vector shift-right.
This patch fixes that.
I've added executable tests for both shift-right algebraic and shift-right
logical.
Both fail prior to applying the patch, and work correctly afterwards.
Minor differences from v1 of this patch:
* Deleted code to defer some vector splats to expand, which was unnecessary.
* Removed powerpc64*-*-* target selector, added -mvsx option, removed extra
braces from dg-options directive.
* Added __attribute__ ((noinline)) to test_s*di_4 functions.
* Corrected typoed function names.
* Changed test case names.
* Added vec-sld-modulo.c as requested (works both before and after this patch).
Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions.
Is
this okay for trunk, and for GCC 8.3 if there is no fallout by the end of the
week?
Thanks,
Bill
[gcc]
2019-02-11 Bill Schmidt <wschm...@linux.ibm.com>
* config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Shift-right
and shift-left vector built-ins need to include a TRUNC_MOD_EXPR
for correct semantics.
[gcc/testsuite]
2019-02-11 Bill Schmidt <wschm...@linux.ibm.com>
* gcc.target/powerpc/vec-sld-modulo.c: New.
* gcc.target/powerpc/vec-srad-modulo.c: New.
* gcc.target/powerpc/vec-srd-modulo.c: New.
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 268707)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -15735,13 +15735,37 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *
case ALTIVEC_BUILTIN_VSRAH:
case ALTIVEC_BUILTIN_VSRAW:
case P8V_BUILTIN_VSRAD:
- arg0 = gimple_call_arg (stmt, 0);
- arg1 = gimple_call_arg (stmt, 1);
- lhs = gimple_call_lhs (stmt);
- g = gimple_build_assign (lhs, RSHIFT_EXPR, arg0, arg1);
- gimple_set_location (g, gimple_location (stmt));
- gsi_replace (gsi, g, true);
- return true;
+ {
+ arg0 = gimple_call_arg (stmt, 0);
+ arg1 = gimple_call_arg (stmt, 1);
+ lhs = gimple_call_lhs (stmt);
+ tree arg1_type = TREE_TYPE (arg1);
+ tree unsigned_arg1_type = unsigned_type_for (TREE_TYPE (arg1));
+ tree unsigned_element_type = unsigned_type_for (TREE_TYPE (arg1_type));
+ location_t loc = gimple_location (stmt);
+ /* Force arg1 into the range valid matching the arg0 type. */
+ /* Build a vector consisting of the max valid bit-size values. */
+ int n_elts = VECTOR_CST_NELTS (arg1);
+ tree element_size = build_int_cst (unsigned_element_type,
+ 128 / n_elts);
+ tree_vector_builder elts (unsigned_arg1_type, n_elts, 1);
+ for (int i = 0; i < n_elts; i++)
+ elts.safe_push (element_size);
+ tree modulo_tree = elts.build ();
+ /* Modulo the provided shift value against that vector. */
+ gimple_seq stmts = NULL;
+ tree unsigned_arg1 = gimple_build (&stmts, VIEW_CONVERT_EXPR,
+ unsigned_arg1_type, arg1);
+ tree new_arg1 = gimple_build (&stmts, loc, TRUNC_MOD_EXPR,
+ unsigned_arg1_type, unsigned_arg1,
+ modulo_tree);
+ gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
+ /* And finally, do the shift. */
+ g = gimple_build_assign (lhs, RSHIFT_EXPR, arg0, new_arg1);
+ gimple_set_location (g, loc);
+ gsi_replace (gsi, g, true);
+ return true;
+ }
/* Flavors of vector shift left.
builtin_altivec_vsl{b,h,w} -> vsl{b,h,w}. */
case ALTIVEC_BUILTIN_VSLB:
@@ -15795,14 +15819,34 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *
arg0 = gimple_call_arg (stmt, 0);
arg1 = gimple_call_arg (stmt, 1);
lhs = gimple_call_lhs (stmt);
+ tree arg1_type = TREE_TYPE (arg1);
+ tree unsigned_arg1_type = unsigned_type_for (TREE_TYPE (arg1));
+ tree unsigned_element_type = unsigned_type_for (TREE_TYPE (arg1_type));
+ location_t loc = gimple_location (stmt);
gimple_seq stmts = NULL;
/* Convert arg0 to unsigned. */
tree arg0_unsigned
= gimple_build (&stmts, VIEW_CONVERT_EXPR,
unsigned_type_for (TREE_TYPE (arg0)), arg0);
+ /* Force arg1 into the range valid matching the arg0 type. */
+ /* Build a vector consisting of the max valid bit-size values. */
+ int n_elts = VECTOR_CST_NELTS (arg1);
+ tree element_size = build_int_cst (unsigned_element_type,
+ 128 / n_elts);
+ tree_vector_builder elts (unsigned_arg1_type, n_elts, 1);
+ for (int i = 0; i < n_elts; i++)
+ elts.safe_push (element_size);
+ tree modulo_tree = elts.build ();
+ /* Modulo the provided shift value against that vector. */
+ tree unsigned_arg1 = gimple_build (&stmts, VIEW_CONVERT_EXPR,
+ unsigned_arg1_type, arg1);
+ tree new_arg1 = gimple_build (&stmts, loc, TRUNC_MOD_EXPR,
+ unsigned_arg1_type, unsigned_arg1,
+ modulo_tree);
+ /* Do the shift. */
tree res
= gimple_build (&stmts, RSHIFT_EXPR,
- TREE_TYPE (arg0_unsigned), arg0_unsigned, arg1);
+ TREE_TYPE (arg0_unsigned), arg0_unsigned, new_arg1);
/* Convert result back to the lhs type. */
res = gimple_build (&stmts, VIEW_CONVERT_EXPR, TREE_TYPE (lhs), res);
gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
Index: gcc/testsuite/gcc.target/powerpc/vec-sld-modulo.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-sld-modulo.c (nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-sld-modulo.c (working copy)
@@ -0,0 +1,42 @@
+/* Test that using a character splat to set up a shift-left
+ for a doubleword vector works correctly after gimple folding. */
+
+/* { dg-do run { target { vsx_hw } } } */
+/* { dg-options "-O2 -mvsx" } */
+
+#include <altivec.h>
+
+typedef __vector unsigned long long vui64_t;
+
+static inline vui64_t
+vec_sldi (vui64_t vra, const unsigned int shb)
+{
+ vui64_t lshift;
+ vui64_t result;
+
+ /* Note legitimate use of wrong-type splat due to expectation that only
+ lower 6-bits are read. */
+ lshift = (vui64_t) vec_splat_s8((const unsigned char)shb);
+
+ /* Vector Shift Left Doublewords based on the lower 6-bits
+ of corresponding element of lshift. */
+ result = vec_vsld (vra, lshift);
+
+ return (vui64_t) result;
+}
+
+__attribute__ ((noinline)) vui64_t
+test_sldi_4 (vui64_t a)
+{
+ return vec_sldi (a, 4);
+}
+
+int
+main ()
+{
+ vui64_t x = {-256, 1025};
+ x = test_sldi_4 (x);
+ if (x[0] != -4096 || x[1] != 16400)
+ __builtin_abort ();
+ return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-srad-modulo.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-srad-modulo.c (nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-srad-modulo.c (working copy)
@@ -0,0 +1,43 @@
+/* Test that using a character splat to set up a shift-right algebraic
+ for a doubleword vector works correctly after gimple folding. */
+
+/* { dg-do run { target { vsx_hw } } } */
+/* { dg-options "-O2 -mvsx" } */
+
+#include <altivec.h>
+
+typedef __vector unsigned long long vui64_t;
+typedef __vector long long vi64_t;
+
+static inline vi64_t
+vec_sradi (vi64_t vra, const unsigned int shb)
+{
+ vui64_t rshift;
+ vi64_t result;
+
+ /* Note legitimate use of wrong-type splat due to expectation that only
+ lower 6-bits are read. */
+ rshift = (vui64_t) vec_splat_s8((const unsigned char)shb);
+
+ /* Vector Shift Right Algebraic Doublewords based on the lower 6-bits
+ of corresponding element of rshift. */
+ result = vec_vsrad (vra, rshift);
+
+ return (vi64_t) result;
+}
+
+__attribute__ ((noinline)) vi64_t
+test_sradi_4 (vi64_t a)
+{
+ return vec_sradi (a, 4);
+}
+
+int
+main ()
+{
+ vi64_t x = {-256, 1025};
+ x = test_sradi_4 (x);
+ if (x[0] != -16 || x[1] != 64)
+ __builtin_abort ();
+ return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-srd-modulo.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-srd-modulo.c (nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-srd-modulo.c (working copy)
@@ -0,0 +1,42 @@
+/* Test that using a character splat to set up a shift-right logical
+ for a doubleword vector works correctly after gimple folding. */
+
+/* { dg-do run { target { vsx_hw } } } */
+/* { dg-options "-O2 -mvsx" } */
+
+#include <altivec.h>
+
+typedef __vector unsigned long long vui64_t;
+
+static inline vui64_t
+vec_srdi (vui64_t vra, const unsigned int shb)
+{
+ vui64_t rshift;
+ vui64_t result;
+
+ /* Note legitimate use of wrong-type splat due to expectation that only
+ lower 6-bits are read. */
+ rshift = (vui64_t) vec_splat_s8((const unsigned char)shb);
+
+ /* Vector Shift Right [Logical] Doublewords based on the lower 6-bits
+ of corresponding element of rshift. */
+ result = vec_vsrd (vra, rshift);
+
+ return (vui64_t) result;
+}
+
+__attribute__ ((noinline)) vui64_t
+test_srdi_4 (vui64_t a)
+{
+ return vec_srdi (a, 4);
+}
+
+int
+main ()
+{
+ vui64_t x = {1992357, 1025};
+ x = test_srdi_4 (x);
+ if (x[0] != 124522 || x[1] != 64)
+ __builtin_abort ();
+ return 0;
+}
