Hi Carl,

On Thu, Nov 09, 2017 at 08:39:10AM -0800, Carl Love wrote:
> The recent commit 254464 to add Power 8 support for the vec_revb()
> builtin had a couple of issues that I missed on my testing of the
> original patch.  First, on Power 8 BE the le_swap1 permute vector value
> was wrong in function swap_endian_selector_for_mode(). This issue caused
> the test case builtins-revb-runnable.c to fail on Power 8 BE. Originally
> the function was used for all of the various modes including V16QI.
> However, a revision of the patch mapped the V16QI case to a move as the
> permute is actually a NOP.  Hence this case is not needed in the
> function.  With these changes, the function values for
> {l,b}e_swap{1,2,4,8} are identical.  Thus the if statement for LE and BE
> can be collapsed.  
> 
> The additional issues were found with an existing testcase p9-xxbr-1.c
> on AIX and Power 9 LE.  The test case was not being properly limited to
> run on Power 9 with the new Power 8 support.  Additionally, the change
> to map the V16QI to a move results means the test no longer generates
> the xxbrq instructions.  The dg directive lp64 is needed on AIX.

Just a few tiny things:

> gcc/ChangeLog:
> 
> 2017-11-09  Carl Love  <c...@us.ibm.com>
> 
>       * config/rs6000/rs6000.c (swap_endian_selector_for_mode): Remove
>       le_ and be_ prefixes to swap* variables.  Remove
>       if (VECTOR_ELT_ORDER_BIG) statement. Remove E_V16QImode case statements.
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-11-09  Carl Love  <c...@us.ibm.com>
> 
>       * builtins-revb-runnable.c (dg-do run): Add lp64 directive. Fix
>       indentation of printf and abort statements.
>       * p9-xxbr-1.c (dg-do compile): Add lp64 && p9vector_h directives.

Two spaces after full stop in changelogs as well.  Typo, "p9vector_hw".

> --- a/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c
> +++ b/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c
> @@ -1,4 +1,4 @@
> -/* { dg-do compile { target { powerpc64*-*-* } } } */
> +/* { dg-do compile { target { powerpc64*-*-*  && { lp64 && p9vector_hw } } } 
> } */

powerpc*-*-* instead?

Okay for trunk.  Thanks!


Segher

Reply via email to