diff mbox series

[rs6000] Fix implementation of vec_packsu (vector unsigned long long, vector unsigned long long) built-in function

Message ID 98281c7a-a9d8-9827-7804-a6c8a9a650fb@linux.ibm.com
State New
Headers show
Series [rs6000] Fix implementation of vec_packsu (vector unsigned long long, vector unsigned long long) built-in function | expand

Commit Message

Kelvin Nilsen June 18, 2018, 4:29 p.m. UTC 
This patch fixes an error in the code generation for vec_packsu (vector unsigned long long, vector unsigned long long).  As previously implemented, this built-in function translates to the vpksdus instruction.

This patch causes vec_packsu (vector unsigned long long, vector unsigned long long) to behave the same as vec_packs (vector unsigned long long, vector unsigned long long) for the same type signature, producing the vpkudus instruction.

This patch has bootstrapped and tested without regressions on powerpc64le-unknown-linux (both P8 and P9) and on powerpc-linux (P8 big-endian, both -m32 and -m64).

Is this ok for the trunk?

gcc/ChangeLog:

2018-06-18  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Change
	behavior of vec_packsu (vector unsigned long long, vector unsigned
	long long) to match behavior of vec_packs with same signature.

gcc/testsuite/ChangeLog:

2018-06-18  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	* gcc.target/powerpc/builtins-1.c: Adjust dg directives to scan
	for vpkudus in place of vpksdus.
	* gcc.target/powerpc/builtins-3-p8.c: Likewise.

Comments

Segher Boessenkool June 19, 2018, 7:30 p.m. UTC | #1 
Hi!

On Mon, Jun 18, 2018 at 11:29:55AM -0500, Kelvin Nilsen wrote:
> +/* A single vpkudus matches twice because this is compiled with -dp,
> +   causing diagnostic comments to appear in the resulting .s file, one
> +   of which matches vpkudus.  */

-dp prints the name of the instruction pattern, which is altivec_vpkudus.
So if you look for the full word instead, this problem isn't there I
think?

> +/* { dg-final { scan-assembler-times "vpkudus" 2 } } */

/* { dg-final { scan-assembler-times {\mvpkudus\M} 1 } } */

Okay with that change (and comment changes).  Thanks!


Segher
Kelvin Nilsen June 22, 2018, 1:30 p.m. UTC | #2 
This has been committed to trunk.

Is this ok to backport to gcc6, gcc7, and gcc8?

Thanks.

On 6/19/18 2:30 PM, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, Jun 18, 2018 at 11:29:55AM -0500, Kelvin Nilsen wrote:
>> +/* A single vpkudus matches twice because this is compiled with -dp,
>> +   causing diagnostic comments to appear in the resulting .s file, one
>> +   of which matches vpkudus.  */
> 
> -dp prints the name of the instruction pattern, which is altivec_vpkudus.
> So if you look for the full word instead, this problem isn't there I
> think?
> 
>> +/* { dg-final { scan-assembler-times "vpkudus" 2 } } */
> 
> /* { dg-final { scan-assembler-times {\mvpkudus\M} 1 } } */
> 
> Okay with that change (and comment changes).  Thanks!
> 
> 
> Segher
> 
>
Segher Boessenkool June 22, 2018, 5:41 p.m. UTC | #3 
On Fri, Jun 22, 2018 at 08:30:12AM -0500, Kelvin Nilsen wrote:
> This has been committed to trunk.
> 
> Is this ok to backport to gcc6, gcc7, and gcc8?

Yes, but wait a few days please.  Thanks!


Segher
diff mbox series

Patch

Index: gcc/config/rs6000/rs6000-c.c
===================================================================
--- gcc/config/rs6000/rs6000-c.c	(revision 261599)
+++ gcc/config/rs6000/rs6000-c.c	(working copy)
@@ -2544,7 +2544,7 @@  const struct altivec_builtin_types altivec_overloa
     RS6000_BTI_unsigned_V8HI, RS6000_BTI_V4SI, RS6000_BTI_V4SI, 0 },
   { ALTIVEC_BUILTIN_VEC_PACKSU, P8V_BUILTIN_VPKSDUS,
     RS6000_BTI_unsigned_V4SI, RS6000_BTI_V2DI, RS6000_BTI_V2DI, 0 },
-  { ALTIVEC_BUILTIN_VEC_PACKSU, P8V_BUILTIN_VPKSDUS,
+  { ALTIVEC_BUILTIN_VEC_PACKSU, P8V_BUILTIN_VPKUDUS,
     RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI, 0 },
   { ALTIVEC_BUILTIN_VEC_VPKSWUS, ALTIVEC_BUILTIN_VPKSWUS,
     RS6000_BTI_unsigned_V8HI, RS6000_BTI_V4SI, RS6000_BTI_V4SI, 0 },
Index: gcc/testsuite/gcc.target/powerpc/builtins-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/builtins-1.c	(revision 261599)
+++ gcc/testsuite/gcc.target/powerpc/builtins-1.c	(working copy)
@@ -297,7 +297,7 @@  int main ()
    vec_mul             mulld | mullw, mulhwu
    vec_nor             xxlnor
    vec_or              xxlor
-   vec_packsu          vpksdus
+   vec_packsu          vpkudus (matches twice due to -dp option)
    vec_                perm vperm
    vec_                round xvrdpi
    vec_sel             xxsel
@@ -335,7 +335,11 @@  int main ()
 /* { dg-final { scan-assembler-times "xxlnor" 6 } } */
 /* { dg-final { scan-assembler-times "xxlor" 11 { target { ilp32 } } } } */
 /* { dg-final { scan-assembler-times "xxlor" 7  { target { lp64 } } } } */
-/* { dg-final { scan-assembler-times "vpksdus" 2 } } */
+
+/* A single vpkudus matches twice because this is compiled with -dp,
+   causing diagnostic comments to appear in the resulting .s file, one
+   of which matches vpkudus.  */
+/* { dg-final { scan-assembler-times "vpkudus" 2 } } */
 /* { dg-final { scan-assembler-times "vperm" 4 } } */
 /* { dg-final { scan-assembler-times "xvrdpi" 2 } } */
 /* { dg-final { scan-assembler-times "xxsel" 10 } } */
Index: gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c	(revision 261599)
+++ gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c	(working copy)
@@ -219,6 +219,8 @@  test_neg_double (vector double x)
      test_vui_packs_vull_vull                  1 vpkudus
      test_vui_packs_vssi_vssi                  1 vpkshss
      test_vsi_packsu_vssi_vssi                 1 vpkshus
+     test_vsi_packsu_vsll_vsll                 1 vpksdus
+     test_vsi_packsu_vull_vull                 1 vpkudus
      test_unsigned_char_popcnt_signed_char     1 vpopcntb
      test_unsigned_char_popcnt_unsigned_char   1 vpopcntb
      test_unsigned_short_popcnt_signed_short   1 vpopcnth
@@ -241,11 +243,11 @@  test_neg_double (vector double x)
 /* { dg-final { scan-assembler-times "vcmpequd" 1 } } */
 /* { dg-final { scan-assembler-times "vpkudum"  1 } } */
 /* { dg-final { scan-assembler-times "vpksdss"  1 } } */
-/* { dg-final { scan-assembler-times "vpkudus"  1 } } */  
+/* { dg-final { scan-assembler-times "vpkudus"  2 } } */  
 /* { dg-final { scan-assembler-times "vpkuhus"  2 } } */
 /* { dg-final { scan-assembler-times "vpkshss"  1 } } */  
 /* { dg-final { scan-assembler-times "vpkshus"  1 } } */  
-/* { dg-final { scan-assembler-times "vpksdus"  2 } } */  
+/* { dg-final { scan-assembler-times "vpksdus"  1 } } */  
 /* { dg-final { scan-assembler-times "vpkuwus"  2 } } */  
 /* { dg-final { scan-assembler-times "vpopcntb" 2 } } */
 /* { dg-final { scan-assembler-times "vpopcnth" 2 } } */