[PATCH v2, rs6000] Fix implementation of vec_pack (vector double, vector double) built-in function
This patch fixes an error in the code generation for vec_pack (vector double,
vector double). As previously implemented, this built-in function translates
to the vpkudum instruction.
This patch causes vec_pack (vector double, vector double) to behave the same as
vec_float2 for the same type signature, producing the vmrgow instruction on
little-endian targets and the vmrgew instruction on big-endian targets.
This revision differs from the initial path submission in that it combines all
of the new testing into two test programs, using target qualifiers on the dg
scan-assembler-times directives.
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-19 Kelvin Nilsen
* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Change
behavior of vec_pack (vector double, vector double) to match
behavior of vec_float2 (vector double, vector double).
gcc/testsuite/ChangeLog:
2018-06-19 Kelvin Nilsen
* gcc.target/powerpc/builtins-3-p8.c (test_pack_float): Remove
this test.
* gcc.target/powerpc/builtins-9.c: New test.
* gcc.target/powerpc/fold-vec-pack-double.c: Modify dg directives
to expect different code generation on big-endian
vs. little-endian targets.
Index: gcc/config/rs6000/rs6000-c.c
===
--- gcc/config/rs6000/rs6000-c.c(revision 261341)
+++ gcc/config/rs6000/rs6000-c.c(working copy)
@@ -2425,7 +2425,7 @@ const struct altivec_builtin_types altivec_overloa
RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V2DI,
RS6000_BTI_unsigned_V2DI, 0 },
{ ALTIVEC_BUILTIN_VEC_PACK, P8V_BUILTIN_VPKUDUM,
RS6000_BTI_bool_V4SI, RS6000_BTI_bool_V2DI, RS6000_BTI_bool_V2DI, 0 },
- { ALTIVEC_BUILTIN_VEC_PACK, P8V_BUILTIN_VPKUDUM,
+ { ALTIVEC_BUILTIN_VEC_PACK, P8V_BUILTIN_FLOAT2_V2DF,
RS6000_BTI_V4SF, RS6000_BTI_V2DF, RS6000_BTI_V2DF, 0 },
{ P8V_BUILTIN_VEC_NEG, P8V_BUILTIN_NEG_V16QI,
Index: gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c
===
--- gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c(revision 261341)
+++ gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c(working copy)
@@ -11,12 +11,6 @@ test_eq_long_long (vector bool long long x, vector
return vec_cmpeq (x, y);
}
-vector float
-test_pack_float (vector double x, vector double y)
-{
- return vec_pack (x, y);
-}
-
vector unsigned char
test_vsi_packs_vusi_vusi (vector unsigned short x,
vector unsigned short y)
@@ -214,7 +208,6 @@ test_neg_double (vector double x)
/* Expected test results:
test_eq_long_long 1 vcmpequd inst
- test_pack_float 1 vpkudum inst
test_vsi_packs_vsll_vsll 1 vpksdss
test_vui_packs_vull_vull 1 vpkudus
test_vui_packs_vssi_vssi 1 vpkshss
@@ -239,7 +232,6 @@ 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 "vpkuhus" 2 } } */
Index: gcc/testsuite/gcc.target/powerpc/builtins-9.c
===
--- gcc/testsuite/gcc.target/powerpc/builtins-9.c (nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/builtins-9.c (working copy)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* Expect same instruction selecton on p8 and above. Fix if future
+ targets behave differently. */
+/* { dg-options "-O3 -maltivec" } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } } */
+
+#include
+
+vector float
+test_pack_float (vector double x, vector double y)
+{
+ return vec_pack (x, y);
+}
+
+/* { dg-final { scan-assembler-times "vmrgew" 1 { target be } } } */
+/* { dg-final { scan-assembler-times "vmrgow" 1 { target le } } } */
+
+/* { dg-final { scan-assembler-times "xvcvdpsp" 2 } } */
+/* { dg-final { scan-assembler-times "xxpermdi" 2 } } */
+
Index: gcc/testsuite/gcc.target/powerpc/fold-vec-pack-double.c
===
--- gcc/testsuite/gcc.target/powerpc/fold-vec-pack-double.c (revision
261341)
+++ gcc/testsuite/gcc.target/powerpc/fold-vec-pack-double.c (working copy)
@@ -3,7 +3,10 @@
/* { dg-do compile } */
/* { dg-require-effective-target powerpc_p8vector_ok } */
-/* { dg-options "-mvsx -mpower8-vector -O2" } */
+/* Expect same instruction selecton on p8 and above. Fix if future
+ targets behave differently.
Re: [PATCH v2, rs6000] Fix implementation of vec_pack (vector double, vector double) built-in function
Hi!
On Tue, Jun 19, 2018 at 01:37:51PM -0500, Kelvin Nilsen wrote:
> --- gcc/testsuite/gcc.target/powerpc/builtins-9.c (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/builtins-9.c (working copy)
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* Expect same instruction selecton on p8 and above. Fix if future
> + targets behave differently. */
> +/* { dg-options "-O3 -maltivec" } */
But this doesn't use -mcpu=power8 or similar. Does it need it anyway?
Both xxpermdi and xvcvdpsp are Power7 (ISA 2.06) and the rest is AltiVec?
So maybe just powerpc_vsx_ok?
> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } } */
You do not use -mcpu= so you don't need this.
Same issues in the next test. Rest looks good though :-)
Segher
Re: [PATCH v2, rs6000] Fix implementation of vec_pack (vector double, vector double) built-in function
Thanks for feedback. It turns out that the vmrgew and vmrgow instructions
require power 8.
After coordinating with Segher on minor refinements to the test cases, I have
committed the patch as quoted below to the trunk.
On 6/19/18 5:37 PM, Segher Boessenkool wrote:
> Hi!
>
> On Tue, Jun 19, 2018 at 01:37:51PM -0500, Kelvin Nilsen wrote:
>> --- gcc/testsuite/gcc.target/powerpc/builtins-9.c(nonexistent)
>> +++ gcc/testsuite/gcc.target/powerpc/builtins-9.c(working copy)
>> @@ -0,0 +1,21 @@
>> +/* { dg-do compile } */
>> +/* { dg-require-effective-target powerpc_p8vector_ok } */
>> +/* Expect same instruction selecton on p8 and above. Fix if future
>> + targets behave differently. */
>> +/* { dg-options "-O3 -maltivec" } */
>
> But this doesn't use -mcpu=power8 or similar. Does it need it anyway?
> Both xxpermdi and xvcvdpsp are Power7 (ISA 2.06) and the rest is AltiVec?
> So maybe just powerpc_vsx_ok?
>
>> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } } */
>
> You do not use -mcpu= so you don't need this.
>
> Same issues in the next test. Rest looks good though :-)
>
>
> Segher
>
>
gcc/ChangeLog:
2018-06-22 Kelvin Nilsen
* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Change
behavior of vec_pack (vector double, vector double) to match
behavior of vec_float2 (vector double, vector double).
gcc/testsuite/ChangeLog:
2018-06-22 Kelvin Nilsen
* gcc.target/powerpc/builtins-3-p8.c (test_pack_float): Remove
this test.
* gcc.target/powerpc/builtins-9.c: New test.
* gcc.target/powerpc/fold-vec-pack-double.c: Modify dg directives
to expect different code generation on big-endian vs.
little-endian targets.
Index: gcc/config/rs6000/rs6000-c.c
===
--- gcc/config/rs6000/rs6000-c.c(revision 261775)
+++ gcc/config/rs6000/rs6000-c.c(working copy)
@@ -2425,7 +2425,7 @@ const struct altivec_builtin_types altivec_overloa
RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V2DI,
RS6000_BTI_unsigned_V2DI, 0 },
{ ALTIVEC_BUILTIN_VEC_PACK, P8V_BUILTIN_VPKUDUM,
RS6000_BTI_bool_V4SI, RS6000_BTI_bool_V2DI, RS6000_BTI_bool_V2DI, 0 },
- { ALTIVEC_BUILTIN_VEC_PACK, P8V_BUILTIN_VPKUDUM,
+ { ALTIVEC_BUILTIN_VEC_PACK, P8V_BUILTIN_FLOAT2_V2DF,
RS6000_BTI_V4SF, RS6000_BTI_V2DF, RS6000_BTI_V2DF, 0 },
{ P8V_BUILTIN_VEC_NEG, P8V_BUILTIN_NEG_V16QI,
Index: gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c
===
--- gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c(revision 261775)
+++ gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c(working copy)
@@ -11,12 +11,6 @@ test_eq_long_long (vector bool long long x, vector
return vec_cmpeq (x, y);
}
-vector float
-test_pack_float (vector double x, vector double y)
-{
- return vec_pack (x, y);
-}
-
vector unsigned char
test_vsi_packs_vusi_vusi (vector unsigned short x,
vector unsigned short y)
@@ -214,7 +208,6 @@ test_neg_double (vector double x)
/* Expected test results:
test_eq_long_long 1 vcmpequd inst
- test_pack_float 1 vpkudum inst
test_vsi_packs_vsll_vsll 1 vpksdss
test_vui_packs_vull_vull 1 vpkudus
test_vui_packs_vssi_vssi 1 vpkshss
@@ -239,7 +232,6 @@ 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 "vpkuhus" 2 } } */
Index: gcc/testsuite/gcc.target/powerpc/builtins-9.c
===
--- gcc/testsuite/gcc.target/powerpc/builtins-9.c (nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/builtins-9.c (working copy)
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power8" } } */
+/* { dg-options "-maltivec -mcpu=power8 -O3" } */
+
+#include
+
+vector float
+test_pack_float (vector double x, vector double y)
+{
+ return vec_pack (x, y);
+}
+
+/* { dg-final { scan-assembler-times "vmrgew" 1 { target be } } } */
+/* { dg-final { scan-assembler-times "vmrgow" 1 { target le } } } */
+
+/* { dg-final { scan-assembler-times "xvcvdpsp" 2 } } */
+/* { dg-final { scan-assembler-times "xxpermdi" 2 } } */
+
Index: gcc/testsuite/gcc.target/powerpc/fold-vec-pack-double.c
===
--- gcc/testsuite/gcc.target/powerpc/fold-vec-pack-double.c (revision
261775)
+++ gcc/testsuite/gcc.target/powerpc/fold-vec-pack-double.c
