Hi Bill,
On Sun, Feb 10, 2019 at 10:10:02AM -0600, Bill Schmidt wrote:
> 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.
Please add a test for left shifts, as well?
> 2019-02-08 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. Also, don't expand a vector-splat if there
> is a type mismatch; let the back end handle it.
Does it always result in just the shift instruction now? Does the modulo
ever remain? (Maybe at -O0?) Modulo is hugely expensive; it is always
modulo by a power of two, so a simple bitmask, so maybe write that directly
instead?
> 2019-02-08 Bill Schmidt <wschm...@linux.ibm.com>
>
> * gcc.target/powerpc/srad-modulo.c: New.
> * gcc.target/powerpc/srd-modulo.c: New.
Please put "vec-" in the testcase name. You may need to rename vec-shift.c
and/or vec-shr.c, which are not as generic as their names suggest.
> @@ -16072,6 +16116,13 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *
> arg0 = gimple_call_arg (stmt, 0);
> lhs = gimple_call_lhs (stmt);
>
> + /* It's sometimes useful to use one of these to build a
> + splat for V2DImode, since the upper bits will be ignored.
> + Don't fold if we detect that situation, as we end up
> + losing the splat instruction in this case. */
> + if (size != TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (TREE_TYPE (lhs)))))
> + return false;
This isn't really detecting that situation... Instead, it doesn't fold
whenever the size of the splatted elements isn't the same as the size of
the elts of the target vector. That's probably perfectly safe, but please
spell it out. It's fine to mention the motivating case, of course.
> Index: gcc/testsuite/gcc.target/powerpc/srad-modulo.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/srad-modulo.c (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/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 { powerpc64*-*-* && vsx_hw } } } */
> +/* { dg-options { "-O3" } } */
powerpc64*-*-* is almost always wrong. I don't think you need to limit
to 64-bit at all here, but if you do, test for lp64 instead.
Testing for vsx_hw but not enabling vsx is probably wrong, too.
Does it need -O3, does -O2 not work?
Should this testcase check expected machine code as well?
> --- gcc/testsuite/gcc.target/powerpc/srd-modulo.c (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/srd-modulo.c (working copy)
> +vui64_t
> +test_sradi_4 (vui64_t a)
> +{
> + return vec_sradi (a, 4);
> +}
Pasto? (srad vs. srd).
Segher
