Re: [PATCH], PR target/67071, Improve easy_altivec_constants on PowerPC

[David Edelsohn <dje.gcc@gmail.com>]

On Mon, Aug 3, 2015 at 5:49 PM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> In preparing the next IEEE 128-bit floating point patch, I needed a quick way
> to load -0.0q into a vector registers (i.e. just the MSB set). I originally had
> a special purpose insn to load this value, but I decided to widen it to allow
> the easy_altivec_constant support to generate constants where you use the
> VSLDOI instruction to create the bottom part of all 0's or all 1's.
>
> When I started doing the coding, I noticed that the current support to load
> vectors with the MSB set in each element no longer worked, because the test
> assumed the constant was stored as an unsigned value, and we now store a sign
> extended number. I raised PR 67071 about this, and this patch fixes that
> problem and adds more patterns of vector constants that can be loaded without
> using memory.
>
> I have built this on both big endian power7 and little endian power8 machines
> with no regressions. Can I install the patch? I would also like to backport
> this patch to the active branches.
>
> [gcc]
> 2015-08-03  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/67071
>         * config/rs6000/predicates.md (easy_vector_constant_vsldoi): New
>         predicate to allow construction of vector constants using the
>         VSLDOI vector shift instruction.
>
>         * config/rs6000/rs6000-protos.h (vspltis_shifted): Add
>         declaration.
>
>         * config/rs6000/rs6000.c (vspltis_shifted): New function to return
>         the number of bytes to be shifted left and filled in with either
>         all zero or all one bits.
>         (gen_easy_altivec_constant): Call vsplitis_shifted if no other
>         methods exist.
>         (output_vec_const_move): On power8, generate XXLORC to generate
>         a vector constant with all 1's. Do a split if we need to use a
>         VSLDOI instruction.
>
>         * config/rs6000/rs6000.h (EASY_VECTOR_MSB): Use mode mask to
>         properly test for the MSB.
>
>         * config/rs6000/altivec.md (VSLDOI splitter): Add splitter for
>         vector constants that can be created with VSLDOI.
>
> [gcc/testsuite]
> 2015-08-03  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/67071
>         * gcc.target/powerpc/pr67071-1.c: New file to test PR 67071 new
>         vector constants.
>         * gcc.target/powerpc/pr67071-2.c: Likewise.
>         * gcc.target/powerpc/pr67071-3.c: Likewise.

This okay, but please fix the formatting of EASY_VECTOR_MSB, which
does not follow GCC formatting style.

 #define EASY_VECTOR_MSB(n,mode) \
-  (((unsigned HOST_WIDE_INT)n) == \
+  ((((unsigned HOST_WIDE_INT)n) & GET_MODE_MASK (mode)) == \
    ((((unsigned HOST_WIDE_INT)GET_MODE_MASK (mode)) + 1) >> 1))

Spaces after the casts and parentheses around the first macro argument "n".

Thanks, David