* [PATCH v2] rs6000: Test case adjustments for new builtins
@ 2021-11-16 20:26 Bill Schmidt
2021-11-17 12:44 ` Segher Boessenkool
0 siblings, 1 reply; 4+ messages in thread
From: Bill Schmidt @ 2021-11-16 20:26 UTC (permalink / raw)
To: GCC Patches; +Cc: Segher Boessenkool, David Edelsohn
Hi! I recently submitted [1] to make adjustments to test cases for the new builtins
support, mostly due to error messages changing for consistency. Thanks for the
previous review. I've reviewed the reasons for the changes and removed unrelated
changes as requested.
A couple of comments:
- For fold-vect-splat-floatdouble.c and fold-vec-splat-longlong.c, the existing
test cases have some bad tests in them (checking two bits when only one bit
is meaningful). The new builtin support catches this but the old support did
not. Removing those bad cases changes some of the scan-assembler-times expected
values.
- For int_128bit-runnable.c, I chose not to do gimple folding on the 128-bit
comparison operations in the new implementation, because doing so results in
bad code that splits things into two 64-bit values. That needs separate
attention; but the point here is, when I did that, I started generating
more of the vcmpequq, vcmpgtsq, and vcmpgtuq instructions.
Everything else here is hopefully straightforward, and unchanged from the previous
submission.
Bootstrapped and tested on powerpc64le-linux-gnu, and on powerpc64-linux-gnu with
-m32 and -m64. Is this okay for trunk?
Thanks!
Bill
[1] https://gcc.gnu.org/pipermail/gcc-patches/2021-September/578615.html
2021-11-15 Bill Schmidt <wschmidt@linux.ibm.com>
gcc/testsuite/
* gcc.target/powerpc/bfp/scalar-extract-exp-2.c: Adjust error
message.
* gcc.target/powerpc/bfp/scalar-extract-sig-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-5.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-8.c: Likewise.
* gcc.target/powerpc/bfp/scalar-test-neg-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-test-neg-3.c: Likewise.
* gcc.target/powerpc/bfp/scalar-test-neg-5.c: Likewise.
* gcc.target/powerpc/byte-in-set-2.c: Likewise.
* gcc.target/powerpc/cmpb-2.c: Likewise.
* gcc.target/powerpc/cmpb32-2.c: Likewise.
* gcc.target/powerpc/crypto-builtin-2.c: Likewise.
* gcc.target/powerpc/fold-vec-splat-floatdouble.c: Remove invalid
test and adjust xxpermdi count.
* gcc.target/powerpc/fold-vec-splat-longlong.c: Remove invalid
tests and adjust instruction counts.
* gcc.target/powerpc/fold-vec-splat-misc-invalid.c: Adjust error
messages.
* gcc.target/powerpc/int_128bit-runnable.c: Adjust instruction
counts since we do better by not gimple-folding some builtins.
* gcc.target/powerpc/pr80315-1.c: Adjust error message.
* gcc.target/powerpc/pr80315-2.c: Likewise.
* gcc.target/powerpc/pr80315-3.c: Likewise.
* gcc.target/powerpc/pr80315-4.c: Likewise.
* gcc.target/powerpc/pr88100.c: Likewise.
* gcc.target/powerpc/pragma_misc9.c: Likewise.
* gcc.target/powerpc/pragma_power8.c: Undef _RS6000_VECDEFINES_H.
* gcc.target/powerpc/pragma_power9.c: Likewise.
* gcc.target/powerpc/test_fpscr_drn_builtin_error.c: Adjust error
messages.
* gcc.target/powerpc/test_fpscr_rn_builtin_error.c: Likewise.
* gcc.target/powerpc/vec-gnb-2.c: Likewise.
* gcc.target/powerpc/vsu/vec-all-nez-7.c: Likewise.
* gcc.target/powerpc/vsu/vec-any-eqz-7.c: Likewise.
* gcc.target/powerpc/vsu/vec-cmpnez-7.c: Likewise.
* gcc.target/powerpc/vsu/vec-cntlz-lsbb-2.c: Likewise.
* gcc.target/powerpc/vsu/vec-cnttz-lsbb-2.c: Likewise.
* gcc.target/powerpc/vsu/vec-xl-len-13.c: Likewise.
* gcc.target/powerpc/vsu/vec-xst-len-12.c: Likewise.
---
| 2 +-
| 2 +-
.../gcc.target/powerpc/bfp/scalar-insert-exp-2.c | 2 +-
.../gcc.target/powerpc/bfp/scalar-insert-exp-5.c | 2 +-
.../gcc.target/powerpc/bfp/scalar-insert-exp-8.c | 2 +-
.../gcc.target/powerpc/bfp/scalar-test-neg-2.c | 2 +-
.../gcc.target/powerpc/bfp/scalar-test-neg-3.c | 2 +-
.../gcc.target/powerpc/bfp/scalar-test-neg-5.c | 2 +-
gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c | 2 +-
gcc/testsuite/gcc.target/powerpc/cmpb-2.c | 2 +-
gcc/testsuite/gcc.target/powerpc/cmpb32-2.c | 2 +-
.../gcc.target/powerpc/crypto-builtin-2.c | 14 +++++++-------
.../powerpc/fold-vec-splat-floatdouble.c | 4 ++--
.../gcc.target/powerpc/fold-vec-splat-longlong.c | 10 +++-------
.../powerpc/fold-vec-splat-misc-invalid.c | 8 ++++----
.../gcc.target/powerpc/int_128bit-runnable.c | 6 +++---
gcc/testsuite/gcc.target/powerpc/pr80315-1.c | 2 +-
gcc/testsuite/gcc.target/powerpc/pr80315-2.c | 2 +-
gcc/testsuite/gcc.target/powerpc/pr80315-3.c | 2 +-
gcc/testsuite/gcc.target/powerpc/pr80315-4.c | 2 +-
gcc/testsuite/gcc.target/powerpc/pr88100.c | 12 ++++++------
gcc/testsuite/gcc.target/powerpc/pragma_misc9.c | 4 ++--
gcc/testsuite/gcc.target/powerpc/pragma_power8.c | 2 ++
gcc/testsuite/gcc.target/powerpc/pragma_power9.c | 3 +++
.../powerpc/test_fpscr_drn_builtin_error.c | 4 ++--
.../powerpc/test_fpscr_rn_builtin_error.c | 12 ++++++------
gcc/testsuite/gcc.target/powerpc/vec-gnb-2.c | 2 +-
.../gcc.target/powerpc/vsu/vec-all-nez-7.c | 2 +-
.../gcc.target/powerpc/vsu/vec-any-eqz-7.c | 2 +-
.../gcc.target/powerpc/vsu/vec-cmpnez-7.c | 2 +-
.../gcc.target/powerpc/vsu/vec-cntlz-lsbb-2.c | 2 +-
.../gcc.target/powerpc/vsu/vec-cnttz-lsbb-2.c | 2 +-
.../gcc.target/powerpc/vsu/vec-xl-len-13.c | 2 +-
.../gcc.target/powerpc/vsu/vec-xst-len-12.c | 2 +-
34 files changed, 63 insertions(+), 62 deletions(-)
--git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c
index 922180675fc..53b67c95cf9 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c
@@ -14,7 +14,7 @@ get_exponent (double *p)
{
double source = *p;
- return scalar_extract_exp (source); /* { dg-error "'__builtin_vec_scalar_extract_exp' is not supported in this compiler configuration" } */
+ return scalar_extract_exp (source); /* { dg-error "'__builtin_vsx_scalar_extract_exp' requires the" } */
}
--git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-2.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-2.c
index e24d4bd23fe..39ee74c94dc 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-2.c
@@ -12,5 +12,5 @@ get_significand (double *p)
{
double source = *p;
- return __builtin_vec_scalar_extract_sig (source); /* { dg-error "'__builtin_vec_scalar_extract_sig' is not supported in this compiler configuration" } */
+ return __builtin_vec_scalar_extract_sig (source); /* { dg-error "'__builtin_vsx_scalar_extract_sig' requires the" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-2.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-2.c
index feb943104da..efd69725905 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-2.c
@@ -16,5 +16,5 @@ insert_exponent (unsigned long long int *significand_p,
unsigned long long int significand = *significand_p;
unsigned long long int exponent = *exponent_p;
- return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vec_scalar_insert_exp' is not supported in this compiler configuration" } */
+ return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vsx_scalar_insert_exp' requires the" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-5.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-5.c
index 0e5683d5d1a..f85966a6fdf 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-5.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-5.c
@@ -16,5 +16,5 @@ insert_exponent (double *significand_p,
double significand = *significand_p;
unsigned long long int exponent = *exponent_p;
- return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vec_scalar_insert_exp' is not supported in this compiler configuration" } */
+ return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vsx_scalar_insert_exp_dp' requires the" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-8.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-8.c
index bd68f770985..b1be8284b4e 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-8.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-8.c
@@ -16,5 +16,5 @@ insert_exponent (unsigned __int128 *significand_p, /* { dg-error "'__int128' is
unsigned __int128 significand = *significand_p; /* { dg-error "'__int128' is not supported on this target" } */
unsigned long long int exponent = *exponent_p;
- return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vec_scalar_insert_exp' is not supported in this compiler configuration" } */
+ return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vsx_scalar_insert_exp' requires the" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-2.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-2.c
index 7d2b4deefc3..46d743a899b 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-2.c
@@ -10,5 +10,5 @@ test_neg (float *p)
{
float source = *p;
- return __builtin_vec_scalar_test_neg_sp (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_sp' requires" } */
+ return __builtin_vec_scalar_test_neg (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_sp' requires" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-3.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-3.c
index b503dfa8b56..bfc892b116e 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-3.c
@@ -10,5 +10,5 @@ test_neg (double *p)
{
double source = *p;
- return __builtin_vec_scalar_test_neg_dp (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_dp' requires" } */
+ return __builtin_vec_scalar_test_neg (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_dp' requires" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-5.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-5.c
index bab86040a7b..8c55c1cfb5c 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-5.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-5.c
@@ -10,5 +10,5 @@ test_neg (__ieee128 *p)
{
__ieee128 source = *p;
- return __builtin_vec_scalar_test_neg_qp (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_qp' requires" } */
+ return __builtin_vec_scalar_test_neg (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_qp' requires" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c b/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c
index 44cc7782760..4c676ba356d 100644
--- a/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c
@@ -10,5 +10,5 @@
int
test_byte_in_set (unsigned char b, unsigned long long set_members)
{
- return __builtin_byte_in_set (b, set_members); /* { dg-warning "implicit declaration of function" } */
+ return __builtin_byte_in_set (b, set_members); /* { dg-error "'__builtin_scalar_byte_in_set' requires the" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/cmpb-2.c b/gcc/testsuite/gcc.target/powerpc/cmpb-2.c
index 113ab6a5f99..02b84d0731d 100644
--- a/gcc/testsuite/gcc.target/powerpc/cmpb-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/cmpb-2.c
@@ -8,7 +8,7 @@ void abort ();
unsigned long long int
do_compare (unsigned long long int a, unsigned long long int b)
{
- return __builtin_cmpb (a, b); /* { dg-warning "implicit declaration of function '__builtin_cmpb'" } */
+ return __builtin_cmpb (a, b); /* { dg-error "'__builtin_p6_cmpb' requires the '-mcpu=power6' option" } */
}
void
diff --git a/gcc/testsuite/gcc.target/powerpc/cmpb32-2.c b/gcc/testsuite/gcc.target/powerpc/cmpb32-2.c
index 37b54745e0e..d4264ab6e7d 100644
--- a/gcc/testsuite/gcc.target/powerpc/cmpb32-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/cmpb32-2.c
@@ -7,7 +7,7 @@ void abort ();
unsigned int
do_compare (unsigned int a, unsigned int b)
{
- return __builtin_cmpb (a, b); /* { dg-warning "implicit declaration of function '__builtin_cmpb'" } */
+ return __builtin_cmpb (a, b); /* { dg-error "'__builtin_p6_cmpb_32' requires the '-mcpu=power6' option" } */
}
void
diff --git a/gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c b/gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c
index 4066b1228dc..b3a6c737a3e 100644
--- a/gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c
@@ -5,21 +5,21 @@
void use_builtins_d (__vector unsigned long long *p, __vector unsigned long long *q, __vector unsigned long long *r, __vector unsigned long long *s)
{
- p[0] = __builtin_crypto_vcipher (q[0], r[0]); /* { dg-error "'__builtin_crypto_vcipher' is not supported with the current options" } */
- p[1] = __builtin_crypto_vcipherlast (q[1], r[1]); /* { dg-error "'__builtin_crypto_vcipherlast' is not supported with the current options" } */
- p[2] = __builtin_crypto_vncipher (q[2], r[2]); /* { dg-error "'__builtin_crypto_vncipher' is not supported with the current options" } */
- p[3] = __builtin_crypto_vncipherlast (q[3], r[3]); /* { dg-error "'__builtin_crypto_vncipherlast' is not supported with the current options" } */
+ p[0] = __builtin_crypto_vcipher (q[0], r[0]); /* { dg-error "'__builtin_crypto_vcipher' requires the '-mcrypto' option" } */
+ p[1] = __builtin_crypto_vcipherlast (q[1], r[1]); /* { dg-error "'__builtin_crypto_vcipherlast' requires the '-mcrypto' option" } */
+ p[2] = __builtin_crypto_vncipher (q[2], r[2]); /* { dg-error "'__builtin_crypto_vncipher' requires the '-mcrypto' option" } */
+ p[3] = __builtin_crypto_vncipherlast (q[3], r[3]); /* { dg-error "'__builtin_crypto_vncipherlast' requires the '-mcrypto' option" } */
p[4] = __builtin_crypto_vpermxor (q[4], r[4], s[4]);
p[5] = __builtin_crypto_vpmsumd (q[5], r[5]);
- p[6] = __builtin_crypto_vshasigmad (q[6], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmad' is not supported with the current options" } */
- p[7] = __builtin_crypto_vsbox (q[7]); /* { dg-error "'__builtin_crypto_vsbox' is not supported with the current options" } */
+ p[6] = __builtin_crypto_vshasigmad (q[6], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmad' requires the '-mcrypto' option" } */
+ p[7] = __builtin_crypto_vsbox (q[7]); /* { dg-error "'__builtin_crypto_vsbox' requires the '-mcrypto' option" } */
}
void use_builtins_w (__vector unsigned int *p, __vector unsigned int *q, __vector unsigned int *r, __vector unsigned int *s)
{
p[0] = __builtin_crypto_vpermxor (q[0], r[0], s[0]);
p[1] = __builtin_crypto_vpmsumw (q[1], r[1]);
- p[2] = __builtin_crypto_vshasigmaw (q[2], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmaw' is not supported with the current options" } */
+ p[2] = __builtin_crypto_vshasigmaw (q[2], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmaw' requires the '-mcrypto' option" } */
}
void use_builtins_h (__vector unsigned short *p, __vector unsigned short *q, __vector unsigned short *r, __vector unsigned short *s)
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-floatdouble.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-floatdouble.c
index 76619177388..b95fa324633 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-floatdouble.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-floatdouble.c
@@ -18,7 +18,7 @@ vector float test_fc ()
vector double testd_00 (vector double x) { return vec_splat (x, 0b00000); }
vector double testd_01 (vector double x) { return vec_splat (x, 0b00001); }
vector double test_dc ()
-{ const vector double y = { 3.0, 5.0 }; return vec_splat (y, 0b00010); }
+{ const vector double y = { 3.0, 5.0 }; return vec_splat (y, 0b00001); }
/* If the source vector is a known constant, we will generate a load or possibly
XXSPLTIW. */
@@ -28,5 +28,5 @@ vector double test_dc ()
/* { dg-final { scan-assembler-times {\mvspltw\M|\mxxspltw\M} 3 } } */
/* For double types, we will generate xxpermdi instructions. */
-/* { dg-final { scan-assembler-times "xxpermdi" 3 } } */
+/* { dg-final { scan-assembler-times "xxpermdi" 2 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c
index b95b987abce..3fa1f05d6f5 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c
@@ -9,23 +9,19 @@
vector bool long long testb_00 (vector bool long long x) { return vec_splat (x, 0b00000); }
vector bool long long testb_01 (vector bool long long x) { return vec_splat (x, 0b00001); }
-vector bool long long testb_02 (vector bool long long x) { return vec_splat (x, 0b00010); }
vector signed long long tests_00 (vector signed long long x) { return vec_splat (x, 0b00000); }
vector signed long long tests_01 (vector signed long long x) { return vec_splat (x, 0b00001); }
-vector signed long long tests_02 (vector signed long long x) { return vec_splat (x, 0b00010); }
vector unsigned long long testu_00 (vector unsigned long long x) { return vec_splat (x, 0b00000); }
vector unsigned long long testu_01 (vector unsigned long long x) { return vec_splat (x, 0b00001); }
-vector unsigned long long testu_02 (vector unsigned long long x) { return vec_splat (x, 0b00010); }
/* Similar test as above, but the source vector is a known constant. */
-vector bool long long test_bll () { const vector bool long long y = {12, 23}; return vec_splat (y, 0b00010); }
-vector signed long long test_sll () { const vector signed long long y = {34, 45}; return vec_splat (y, 0b00010); }
-vector unsigned long long test_ull () { const vector unsigned long long y = {56, 67}; return vec_splat (y, 0b00010); }
+vector bool long long test_bll () { const vector bool long long y = {12, 23}; return vec_splat (y, 0b00001); }
+vector signed long long test_sll () { const vector signed long long y = {34, 45}; return vec_splat (y, 0b00001); }
/* Assorted load instructions for the initialization with known constants. */
-/* { dg-final { scan-assembler-times {\mlvx\M|\mlxvd2x\M|\mlxv\M|\mplxv\M} 3 } } */
+/* { dg-final { scan-assembler-times {\mlvx\M|\mlxvd2x\M|\mlxv\M|\mplxv\M|\mxxspltib\M} 2 } } */
/* xxpermdi for vec_splat of long long vectors.
At the time of this writing, the number of xxpermdi instructions
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-misc-invalid.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-misc-invalid.c
index 20f5b05561e..263a1723d31 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-misc-invalid.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-misc-invalid.c
@@ -10,24 +10,24 @@
vector signed short
testss_1 (unsigned int ui)
{
- return vec_splat_s16 (ui);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_s16 (ui);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
vector unsigned short
testss_2 (signed int si)
{
- return vec_splat_u16 (si);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_u16 (si);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
vector signed char
testsc_1 (unsigned int ui)
{
- return vec_splat_s8 (ui); /* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_s8 (ui); /* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
vector unsigned char
testsc_2 (signed int si)
{
- return vec_splat_u8 (si);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_u8 (si);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
index 1255ee9f0ab..1356793635a 100644
--- a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
+++ b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
@@ -11,9 +11,9 @@
/* { dg-final { scan-assembler-times {\mvrlq\M} 2 } } */
/* { dg-final { scan-assembler-times {\mvrlqnm\M} 2 } } */
/* { dg-final { scan-assembler-times {\mvrlqmi\M} 2 } } */
-/* { dg-final { scan-assembler-times {\mvcmpequq\M} 16 } } */
-/* { dg-final { scan-assembler-times {\mvcmpgtsq\M} 16 } } */
-/* { dg-final { scan-assembler-times {\mvcmpgtuq\M} 16 } } */
+/* { dg-final { scan-assembler-times {\mvcmpequq\M} 24 } } */
+/* { dg-final { scan-assembler-times {\mvcmpgtsq\M} 26 } } */
+/* { dg-final { scan-assembler-times {\mvcmpgtuq\M} 26 } } */
/* { dg-final { scan-assembler-times {\mvmuloud\M} 1 } } */
/* { dg-final { scan-assembler-times {\mvmulesd\M} 1 } } */
/* { dg-final { scan-assembler-times {\mvmulosd\M} 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-1.c b/gcc/testsuite/gcc.target/powerpc/pr80315-1.c
index e2db0ff4b5f..f37f1f169a2 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80315-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80315-1.c
@@ -10,6 +10,6 @@ main()
int mask;
/* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */
- res = __builtin_crypto_vshasigmaw (test, 1, 0xff); /* { dg-error {argument 3 must be in the range \[0, 15\]} } */
+ res = __builtin_crypto_vshasigmaw (test, 1, 0xff); /* { dg-error {argument 3 must be a 4-bit unsigned literal} } */
return 0;
}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-2.c b/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
index 144b705c012..0819a0511b7 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
@@ -10,6 +10,6 @@ main ()
int mask;
/* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */
- res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error {argument 3 must be in the range \[0, 15\]} } */
+ res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error {argument 3 must be a 4-bit unsigned literal} } */
return 0;
}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-3.c b/gcc/testsuite/gcc.target/powerpc/pr80315-3.c
index 99a3e24eadd..cc2e46cf5cb 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80315-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80315-3.c
@@ -12,6 +12,6 @@ main ()
int mask;
/* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */
- res = vec_shasigma_be (test, 1, 0xff); /* { dg-error {argument 3 must be in the range \[0, 15\]} } */
+ res = vec_shasigma_be (test, 1, 0xff); /* { dg-error {argument 3 must be a 4-bit unsigned literal} } */
return res;
}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-4.c b/gcc/testsuite/gcc.target/powerpc/pr80315-4.c
index 7f5f6f75029..ac12910741b 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80315-4.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80315-4.c
@@ -12,6 +12,6 @@ main ()
int mask;
/* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */
- res = vec_shasigma_be (test, 1, 0xff); /* { dg-error {argument 3 must be in the range \[0, 15\]} } */
+ res = vec_shasigma_be (test, 1, 0xff); /* { dg-error {argument 3 must be a 4-bit unsigned literal} } */
return res;
}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr88100.c b/gcc/testsuite/gcc.target/powerpc/pr88100.c
index 4452145ce95..764c897a497 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr88100.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr88100.c
@@ -10,35 +10,35 @@
vector unsigned char
splatu1 (void)
{
- return vec_splat_u8(0x100);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_u8(0x100);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
vector unsigned short
splatu2 (void)
{
- return vec_splat_u16(0x10000);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_u16(0x10000);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
vector unsigned int
splatu3 (void)
{
- return vec_splat_u32(0x10000000);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_u32(0x10000000);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
vector signed char
splats1 (void)
{
- return vec_splat_s8(0x100);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_s8(0x100);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
vector signed short
splats2 (void)
{
- return vec_splat_s16(0x10000);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_s16(0x10000);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
vector signed int
splats3 (void)
{
- return vec_splat_s32(0x10000000);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
+ return vec_splat_s32(0x10000000);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/pragma_misc9.c b/gcc/testsuite/gcc.target/powerpc/pragma_misc9.c
index e03099bd084..c1667d9f7db 100644
--- a/gcc/testsuite/gcc.target/powerpc/pragma_misc9.c
+++ b/gcc/testsuite/gcc.target/powerpc/pragma_misc9.c
@@ -20,7 +20,7 @@ vector bool int
test2 (vector signed int a, vector signed int b)
{
return vec_cmpnez (a, b);
- /* { dg-error "'__builtin_altivec_vcmpnezw' requires the '-mcpu=power9' option" "" { target *-*-* } .-1 } */
+ /* { dg-error "'__builtin_altivec_vcmpnezw' requires the '-mcpu=power9' and '-mvsx' options" "" { target *-*-* } .-1 } */
}
#pragma GCC target ("cpu=power7")
@@ -28,7 +28,7 @@ vector signed int
test3 (vector signed int a, vector signed int b)
{
return vec_mergee (a, b);
- /* { dg-error "'__builtin_altivec_vmrgew_v4si' requires the '-mpower8-vector' option" "" { target *-*-* } .-1 } */
+ /* { dg-error "'__builtin_altivec_vmrgew_v4si' requires the '-mcpu=power8' and '-mvsx' options" "" { target *-*-* } .-1 } */
}
#pragma GCC target ("cpu=power6")
diff --git a/gcc/testsuite/gcc.target/powerpc/pragma_power8.c b/gcc/testsuite/gcc.target/powerpc/pragma_power8.c
index c8d2cdd6c1a..cb0f30844d3 100644
--- a/gcc/testsuite/gcc.target/powerpc/pragma_power8.c
+++ b/gcc/testsuite/gcc.target/powerpc/pragma_power8.c
@@ -19,6 +19,7 @@ test1 (vector int a, vector int b)
#pragma GCC target ("cpu=power7")
/* Force a re-read of altivec.h with new cpu target. */
#undef _ALTIVEC_H
+#undef _RS6000_VECDEFINES_H
#include <altivec.h>
#ifdef _ARCH_PWR7
vector signed int
@@ -33,6 +34,7 @@ test2 (vector signed int a, vector signed int b)
#pragma GCC target ("cpu=power8")
/* Force a re-read of altivec.h with new cpu target. */
#undef _ALTIVEC_H
+#undef _RS6000_VECDEFINES_H
#include <altivec.h>
#ifdef _ARCH_PWR8
vector int
diff --git a/gcc/testsuite/gcc.target/powerpc/pragma_power9.c b/gcc/testsuite/gcc.target/powerpc/pragma_power9.c
index e33aad1aaf7..e05f1f4ddfa 100644
--- a/gcc/testsuite/gcc.target/powerpc/pragma_power9.c
+++ b/gcc/testsuite/gcc.target/powerpc/pragma_power9.c
@@ -17,6 +17,7 @@ test1 (vector int a, vector int b)
#pragma GCC target ("cpu=power7")
#undef _ALTIVEC_H
+#undef _RS6000_VECDEFINES_H
#include <altivec.h>
#ifdef _ARCH_PWR7
vector signed int
@@ -30,6 +31,7 @@ test2 (vector signed int a, vector signed int b)
#pragma GCC target ("cpu=power8")
#undef _ALTIVEC_H
+#undef _RS6000_VECDEFINES_H
#include <altivec.h>
#ifdef _ARCH_PWR8
vector int
@@ -50,6 +52,7 @@ test3b (vec_t a, vec_t b)
#pragma GCC target ("cpu=power9,power9-vector")
#undef _ALTIVEC_H
+#undef _RS6000_VECDEFINES_H
#include <altivec.h>
#ifdef _ARCH_PWR9
vector bool int
diff --git a/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c b/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c
index 028ab0b6d66..4f9d9e08e8a 100644
--- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c
+++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c
@@ -9,8 +9,8 @@ int main ()
__builtin_set_fpscr_drn() also support a variable as an argument but
can't test variable value at compile time. */
- __builtin_set_fpscr_drn(-1); /* { dg-error "Argument must be a value between 0 and 7" } */
- __builtin_set_fpscr_drn(8); /* { dg-error "Argument must be a value between 0 and 7" } */
+ __builtin_set_fpscr_drn(-1); /* { dg-error "argument 1 must be a variable or a literal between 0 and 7, inclusive" } */
+ __builtin_set_fpscr_drn(8); /* { dg-error "argument 1 must be a variable or a literal between 0 and 7, inclusive" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
index aea65091b0c..10391b71008 100644
--- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
+++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
@@ -8,13 +8,13 @@ int main ()
int arguments. The builtins __builtin_set_fpscr_rn() also supports a
variable as an argument but can't test variable value at compile time. */
- __builtin_mtfsb0(-1); /* { dg-error "Argument must be a constant between 0 and 31" } */
- __builtin_mtfsb0(32); /* { dg-error "Argument must be a constant between 0 and 31" } */
+ __builtin_mtfsb0(-1); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
+ __builtin_mtfsb0(32); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
- __builtin_mtfsb1(-1); /* { dg-error "Argument must be a constant between 0 and 31" } */
- __builtin_mtfsb1(32); /* { dg-error "Argument must be a constant between 0 and 31" } */
+ __builtin_mtfsb1(-1); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
+ __builtin_mtfsb1(32); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
- __builtin_set_fpscr_rn(-1); /* { dg-error "Argument must be a value between 0 and 3" } */
- __builtin_set_fpscr_rn(4); /* { dg-error "Argument must be a value between 0 and 3" } */
+ __builtin_set_fpscr_rn(-1); /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */
+ __builtin_set_fpscr_rn(4); /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/vec-gnb-2.c b/gcc/testsuite/gcc.target/powerpc/vec-gnb-2.c
index 895bb953b37..4e59cbffa17 100644
--- a/gcc/testsuite/gcc.target/powerpc/vec-gnb-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/vec-gnb-2.c
@@ -20,7 +20,7 @@ do_vec_gnb (vector unsigned __int128 source, int stride)
case 5:
return vec_gnb (source, 1); /* { dg-error "between 2 and 7" } */
case 6:
- return vec_gnb (source, stride); /* { dg-error "unsigned literal" } */
+ return vec_gnb (source, stride); /* { dg-error "literal" } */
case 7:
return vec_gnb (source, 7);
diff --git a/gcc/testsuite/gcc.target/powerpc/vsu/vec-all-nez-7.c b/gcc/testsuite/gcc.target/powerpc/vsu/vec-all-nez-7.c
index f53c6dca0a9..a41e82e3b8c 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsu/vec-all-nez-7.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsu/vec-all-nez-7.c
@@ -12,5 +12,5 @@ test_all_not_equal_and_not_zero (vector unsigned short *arg1_p,
vector unsigned short arg_2 = *arg2_p;
return __builtin_vec_vcmpnez_p (__CR6_LT, arg_1, arg_2);
- /* { dg-error "'__builtin_altivec_vcmpnezh_p' requires the '-mcpu=power9' option" "" { target *-*-* } .-1 } */
+ /* { dg-error "'__builtin_altivec_vcmpnezh_p' requires the '-mcpu=power9' and '-mvsx' options" "" { target *-*-* } .-1 } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/vsu/vec-any-eqz-7.c b/gcc/testsuite/gcc.target/powerpc/vsu/vec-any-eqz-7.c
index 757acd93110..3bf8a324772 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsu/vec-any-eqz-7.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsu/vec-any-eqz-7.c
@@ -11,5 +11,5 @@ test_any_equal (vector unsigned int *arg1_p, vector unsigned int *arg2_p)
vector unsigned int arg_2 = *arg2_p;
return __builtin_vec_vcmpnez_p (__CR6_LT_REV, arg_1, arg_2);
- /* { dg-error "'__builtin_altivec_vcmpnezw_p' requires the '-mcpu=power9' option" "" { target *-*-* } .-1 } */
+ /* { dg-error "'__builtin_altivec_vcmpnezw_p' requires the '-mcpu=power9' and '-mvsx' options" "" { target *-*-* } .-1 } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/vsu/vec-cmpnez-7.c b/gcc/testsuite/gcc.target/powerpc/vsu/vec-cmpnez-7.c
index 811b32f1c32..52110af7283 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsu/vec-cmpnez-7.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsu/vec-cmpnez-7.c
@@ -10,5 +10,5 @@ fetch_data (vector unsigned int *arg1_p, vector unsigned int *arg2_p)
vector unsigned int arg_1 = *arg1_p;
vector unsigned int arg_2 = *arg2_p;
- return __builtin_vec_vcmpnez (arg_1, arg_2); /* { dg-error "'__builtin_altivec_vcmpnezw' requires the '-mcpu=power9' option" } */
+ return __builtin_vec_vcmpnez (arg_1, arg_2); /* { dg-error "'__builtin_altivec_vcmpnezw' requires the '-mcpu=power9' and '-mvsx' options" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/vsu/vec-cntlz-lsbb-2.c b/gcc/testsuite/gcc.target/powerpc/vsu/vec-cntlz-lsbb-2.c
index 6ee066d1eff..dd0d3374879 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsu/vec-cntlz-lsbb-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsu/vec-cntlz-lsbb-2.c
@@ -9,5 +9,5 @@ count_leading_zero_byte_bits (vector unsigned char *arg1_p)
{
vector unsigned char arg_1 = *arg1_p;
- return __builtin_vec_vclzlsbb (arg_1); /* { dg-error "'__builtin_altivec_vclzlsbb_v16qi' requires the '-mcpu=power9' option" } */
+ return __builtin_vec_vclzlsbb (arg_1); /* { dg-error "'__builtin_altivec_vclzlsbb_v16qi' requires the '-mcpu=power9' and '-mvsx' options" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/vsu/vec-cnttz-lsbb-2.c b/gcc/testsuite/gcc.target/powerpc/vsu/vec-cnttz-lsbb-2.c
index ecd0add70d0..22c55ef25ae 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsu/vec-cnttz-lsbb-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsu/vec-cnttz-lsbb-2.c
@@ -9,5 +9,5 @@ count_trailing_zero_byte_bits (vector unsigned char *arg1_p)
{
vector unsigned char arg_1 = *arg1_p;
- return __builtin_vec_vctzlsbb (arg_1); /* { dg-error "'__builtin_altivec_vctzlsbb_v16qi' requires the '-mcpu=power9' option" } */
+ return __builtin_vec_vctzlsbb (arg_1); /* { dg-error "'__builtin_altivec_vctzlsbb_v16qi' requires the '-mcpu=power9' and '-mvsx' options" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/vsu/vec-xl-len-13.c b/gcc/testsuite/gcc.target/powerpc/vsu/vec-xl-len-13.c
index 1cfed57d6a6..0f601fbbb50 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsu/vec-xl-len-13.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsu/vec-xl-len-13.c
@@ -13,5 +13,5 @@
int
fetch_data (float *address, size_t length)
{
- return __builtin_vec_lxvl (address, length); /* { dg-warning "'__builtin_vec_lxvl'" } */
+ return __builtin_vec_lxvl (address, length); /* { dg-error "'__builtin_vsx_lxvl' requires the" } */
}
diff --git a/gcc/testsuite/gcc.target/powerpc/vsu/vec-xst-len-12.c b/gcc/testsuite/gcc.target/powerpc/vsu/vec-xst-len-12.c
index 3a51132a5a2..f30d49cb4cc 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsu/vec-xst-len-12.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsu/vec-xst-len-12.c
@@ -13,5 +13,5 @@ store_data (vector double *datap, double *address, size_t length)
{
vector double data = *datap;
- __builtin_vec_stxvl (data, address, length); /* { dg-error "'__builtin_vec_stxvl' is not supported in this compiler configuration" } */
+ __builtin_vec_stxvl (data, address, length); /* { dg-error "'__builtin_altivec_stxvl' requires the" } */
}
--
2.27.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] rs6000: Test case adjustments for new builtins
2021-11-16 20:26 [PATCH v2] rs6000: Test case adjustments for new builtins Bill Schmidt
@ 2021-11-17 12:44 ` Segher Boessenkool
2021-11-17 13:52 ` Bill Schmidt
0 siblings, 1 reply; 4+ messages in thread
From: Segher Boessenkool @ 2021-11-17 12:44 UTC (permalink / raw)
To: Bill Schmidt; +Cc: GCC Patches, David Edelsohn
Hi!
On Tue, Nov 16, 2021 at 02:26:22PM -0600, Bill Schmidt wrote:
> Hi! I recently submitted [1] to make adjustments to test cases for the new builtins
> support, mostly due to error messages changing for consistency. Thanks for the
> previous review. I've reviewed the reasons for the changes and removed unrelated
> changes as requested.
And the results are? This is much easier to write up, and to review, if
you split the patch into pieces with one theme each. If you do that
right then most reviews will be rubber-stamping, and some might require
some thought (and some may even get objections). The way things are it
is a puzzle hunt to review this.
> - For fold-vect-splat-floatdouble.c and fold-vec-splat-longlong.c, the existing
> test cases have some bad tests in them (checking two bits when only one bit
> is meaningful). The new builtin support catches this but the old support did
> not. Removing those bad cases changes some of the scan-assembler-times expected
> values.
Do this is a separate patch then, independent of the series? With this
explanation in the commit message. This is pre-approved.
> - For int_128bit-runnable.c, I chose not to do gimple folding on the 128-bit
> comparison operations in the new implementation, because doing so results in
> bad code that splits things into two 64-bit values. That needs separate
> attention; but the point here is, when I did that, I started generating
> more of the vcmpequq, vcmpgtsq, and vcmpgtuq instructions.
And you now get worse code (albeit in some cases no longer invalid)?
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c
> @@ -14,7 +14,7 @@ get_exponent (double *p)
> {
> double source = *p;
>
> - return scalar_extract_exp (source); /* { dg-error "'__builtin_vec_scalar_extract_exp' is not supported in this compiler configuration" } */
> + return scalar_extract_exp (source); /* { dg-error "'__builtin_vsx_scalar_extract_exp' requires the" } */
> }
The testcase uses __builtin_vec_scalar_extract_exp, so this is not okay.
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-2.c
> @@ -12,5 +12,5 @@ get_significand (double *p)
> {
> double source = *p;
>
> - return __builtin_vec_scalar_extract_sig (source); /* { dg-error "'__builtin_vec_scalar_extract_sig' is not supported in this compiler configuration" } */
> + return __builtin_vec_scalar_extract_sig (source); /* { dg-error "'__builtin_vsx_scalar_extract_sig' requires the" } */
> }
This not either.
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-2.c
> @@ -16,5 +16,5 @@ insert_exponent (unsigned long long int *significand_p,
> unsigned long long int significand = *significand_p;
> unsigned long long int exponent = *exponent_p;
>
> - return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vec_scalar_insert_exp' is not supported in this compiler configuration" } */
> + return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vsx_scalar_insert_exp' requires the" } */
Or this.
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-5.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-5.c
> @@ -16,5 +16,5 @@ insert_exponent (double *significand_p,
> double significand = *significand_p;
> unsigned long long int exponent = *exponent_p;
>
> - return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vec_scalar_insert_exp' is not supported in this compiler configuration" } */
> + return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vsx_scalar_insert_exp_dp' requires the" } */
> }
Etc.
It is not okay to blindly adjust the testcases to accept what the new
code does. This is a regression. It is okay to have it regressed for a
while. It is also okay to xfail things, if there is no expectation it
can be fixed before the next release (or some other suitably big time
frame, this isn't an exact science).
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-2.c
> @@ -10,5 +10,5 @@ test_neg (float *p)
> {
> float source = *p;
>
> - return __builtin_vec_scalar_test_neg_sp (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_sp' requires" } */
> + return __builtin_vec_scalar_test_neg (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_sp' requires" } */
> }
This one is very curious. You change the test to use a more generic
builtin name, presumably because the (undocumented) more specific name
is no longer allowed, but the error message still uses that name?
> --- a/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c
> @@ -10,5 +10,5 @@
> int
> test_byte_in_set (unsigned char b, unsigned long long set_members)
> {
> - return __builtin_byte_in_set (b, set_members); /* { dg-warning "implicit declaration of function" } */
> + return __builtin_byte_in_set (b, set_members); /* { dg-error "'__builtin_scalar_byte_in_set' requires the" } */
> }
Huh. How can the old warning ever have fired? Was the builtin not
declared on 32-bit before? Ouch.
> --- a/gcc/testsuite/gcc.target/powerpc/cmpb-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/cmpb-2.c
> @@ -8,7 +8,7 @@ void abort ();
> unsigned long long int
> do_compare (unsigned long long int a, unsigned long long int b)
> {
> - return __builtin_cmpb (a, b); /* { dg-warning "implicit declaration of function '__builtin_cmpb'" } */
> + return __builtin_cmpb (a, b); /* { dg-error "'__builtin_p6_cmpb' requires the '-mcpu=power6' option" } */
> }
We talked about this one before already.
> --- a/gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c
> @@ -5,21 +5,21 @@
>
> void use_builtins_d (__vector unsigned long long *p, __vector unsigned long long *q, __vector unsigned long long *r, __vector unsigned long long *s)
> {
> - p[0] = __builtin_crypto_vcipher (q[0], r[0]); /* { dg-error "'__builtin_crypto_vcipher' is not supported with the current options" } */
> - p[1] = __builtin_crypto_vcipherlast (q[1], r[1]); /* { dg-error "'__builtin_crypto_vcipherlast' is not supported with the current options" } */
> - p[2] = __builtin_crypto_vncipher (q[2], r[2]); /* { dg-error "'__builtin_crypto_vncipher' is not supported with the current options" } */
> - p[3] = __builtin_crypto_vncipherlast (q[3], r[3]); /* { dg-error "'__builtin_crypto_vncipherlast' is not supported with the current options" } */
> + p[0] = __builtin_crypto_vcipher (q[0], r[0]); /* { dg-error "'__builtin_crypto_vcipher' requires the '-mcrypto' option" } */
> + p[1] = __builtin_crypto_vcipherlast (q[1], r[1]); /* { dg-error "'__builtin_crypto_vcipherlast' requires the '-mcrypto' option" } */
> + p[2] = __builtin_crypto_vncipher (q[2], r[2]); /* { dg-error "'__builtin_crypto_vncipher' requires the '-mcrypto' option" } */
> + p[3] = __builtin_crypto_vncipherlast (q[3], r[3]); /* { dg-error "'__builtin_crypto_vncipherlast' requires the '-mcrypto' option" } */
> p[4] = __builtin_crypto_vpermxor (q[4], r[4], s[4]);
> p[5] = __builtin_crypto_vpmsumd (q[5], r[5]);
> - p[6] = __builtin_crypto_vshasigmad (q[6], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmad' is not supported with the current options" } */
> - p[7] = __builtin_crypto_vsbox (q[7]); /* { dg-error "'__builtin_crypto_vsbox' is not supported with the current options" } */
> + p[6] = __builtin_crypto_vshasigmad (q[6], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmad' requires the '-mcrypto' option" } */
> + p[7] = __builtin_crypto_vsbox (q[7]); /* { dg-error "'__builtin_crypto_vsbox' requires the '-mcrypto' option" } */
> }
>
> void use_builtins_w (__vector unsigned int *p, __vector unsigned int *q, __vector unsigned int *r, __vector unsigned int *s)
> {
> p[0] = __builtin_crypto_vpermxor (q[0], r[0], s[0]);
> p[1] = __builtin_crypto_vpmsumw (q[1], r[1]);
> - p[2] = __builtin_crypto_vshasigmaw (q[2], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmaw' is not supported with the current options" } */
> + p[2] = __builtin_crypto_vshasigmaw (q[2], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmaw' requires the '-mcrypto' option" } */
> }
This one is fine.
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-floatdouble.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-floatdouble.c
> @@ -18,7 +18,7 @@ vector float test_fc ()
> vector double testd_00 (vector double x) { return vec_splat (x, 0b00000); }
> vector double testd_01 (vector double x) { return vec_splat (x, 0b00001); }
> vector double test_dc ()
> -{ const vector double y = { 3.0, 5.0 }; return vec_splat (y, 0b00010); }
> +{ const vector double y = { 3.0, 5.0 }; return vec_splat (y, 0b00001); }
>
> /* If the source vector is a known constant, we will generate a load or possibly
> XXSPLTIW. */
> @@ -28,5 +28,5 @@ vector double test_dc ()
> /* { dg-final { scan-assembler-times {\mvspltw\M|\mxxspltw\M} 3 } } */
>
> /* For double types, we will generate xxpermdi instructions. */
> -/* { dg-final { scan-assembler-times "xxpermdi" 3 } } */
> +/* { dg-final { scan-assembler-times "xxpermdi" 2 } } */
This is okay as a separate patch, with proper commit message.
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c
> @@ -9,23 +9,19 @@
>
> vector bool long long testb_00 (vector bool long long x) { return vec_splat (x, 0b00000); }
> vector bool long long testb_01 (vector bool long long x) { return vec_splat (x, 0b00001); }
> -vector bool long long testb_02 (vector bool long long x) { return vec_splat (x, 0b00010); }
>
> vector signed long long tests_00 (vector signed long long x) { return vec_splat (x, 0b00000); }
> vector signed long long tests_01 (vector signed long long x) { return vec_splat (x, 0b00001); }
> -vector signed long long tests_02 (vector signed long long x) { return vec_splat (x, 0b00010); }
>
> vector unsigned long long testu_00 (vector unsigned long long x) { return vec_splat (x, 0b00000); }
> vector unsigned long long testu_01 (vector unsigned long long x) { return vec_splat (x, 0b00001); }
> -vector unsigned long long testu_02 (vector unsigned long long x) { return vec_splat (x, 0b00010); }
>
> /* Similar test as above, but the source vector is a known constant. */
> -vector bool long long test_bll () { const vector bool long long y = {12, 23}; return vec_splat (y, 0b00010); }
> -vector signed long long test_sll () { const vector signed long long y = {34, 45}; return vec_splat (y, 0b00010); }
> -vector unsigned long long test_ull () { const vector unsigned long long y = {56, 67}; return vec_splat (y, 0b00010); }
> +vector bool long long test_bll () { const vector bool long long y = {12, 23}; return vec_splat (y, 0b00001); }
> +vector signed long long test_sll () { const vector signed long long y = {34, 45}; return vec_splat (y, 0b00001); }
>
> /* Assorted load instructions for the initialization with known constants. */
> -/* { dg-final { scan-assembler-times {\mlvx\M|\mlxvd2x\M|\mlxv\M|\mplxv\M} 3 } } */
> +/* { dg-final { scan-assembler-times {\mlvx\M|\mlxvd2x\M|\mlxv\M|\mplxv\M|\mxxspltib\M} 2 } } */
Ditto.
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-misc-invalid.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-misc-invalid.c
> @@ -10,24 +10,24 @@
> vector signed short
> testss_1 (unsigned int ui)
> {
> - return vec_splat_s16 (ui);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
> + return vec_splat_s16 (ui);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
> }
All such things are fine of course.
> --- a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
> @@ -11,9 +11,9 @@
> /* { dg-final { scan-assembler-times {\mvrlq\M} 2 } } */
> /* { dg-final { scan-assembler-times {\mvrlqnm\M} 2 } } */
> /* { dg-final { scan-assembler-times {\mvrlqmi\M} 2 } } */
> -/* { dg-final { scan-assembler-times {\mvcmpequq\M} 16 } } */
> -/* { dg-final { scan-assembler-times {\mvcmpgtsq\M} 16 } } */
> -/* { dg-final { scan-assembler-times {\mvcmpgtuq\M} 16 } } */
> +/* { dg-final { scan-assembler-times {\mvcmpequq\M} 24 } } */
> +/* { dg-final { scan-assembler-times {\mvcmpgtsq\M} 26 } } */
> +/* { dg-final { scan-assembler-times {\mvcmpgtuq\M} 26 } } */
> /* { dg-final { scan-assembler-times {\mvmuloud\M} 1 } } */
> /* { dg-final { scan-assembler-times {\mvmulesd\M} 1 } } */
> /* { dg-final { scan-assembler-times {\mvmulosd\M} 1 } } */
This either needs more explanation (and be a separate patch), or it is
just wrong :-(
> --- a/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
> @@ -10,6 +10,6 @@ main ()
> int mask;
>
> /* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */
> - res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error {argument 3 must be in the range \[0, 15\]} } */
> + res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error {argument 3 must be a 4-bit unsigned literal} } */
> return 0;
> }
Hrm, make this say "must be a literal between 0 and 15, inclusive" like
the other errors?
> --- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
> +++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
> @@ -8,13 +8,13 @@ int main ()
> int arguments. The builtins __builtin_set_fpscr_rn() also supports a
> variable as an argument but can't test variable value at compile time. */
>
> - __builtin_mtfsb0(-1); /* { dg-error "Argument must be a constant between 0 and 31" } */
> - __builtin_mtfsb0(32); /* { dg-error "Argument must be a constant between 0 and 31" } */
> + __builtin_mtfsb0(-1); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
> + __builtin_mtfsb0(32); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
>
> - __builtin_mtfsb1(-1); /* { dg-error "Argument must be a constant between 0 and 31" } */
> - __builtin_mtfsb1(32); /* { dg-error "Argument must be a constant between 0 and 31" } */
> + __builtin_mtfsb1(-1); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
> + __builtin_mtfsb1(32); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
>
> - __builtin_set_fpscr_rn(-1); /* { dg-error "Argument must be a value between 0 and 3" } */
> - __builtin_set_fpscr_rn(4); /* { dg-error "Argument must be a value between 0 and 3" } */
> + __builtin_set_fpscr_rn(-1); /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */
> + __builtin_set_fpscr_rn(4); /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */
> }
This regressed as well.
> --- a/gcc/testsuite/gcc.target/powerpc/vec-gnb-2.c
> +++ b/gcc/testsuite/gcc.target/powerpc/vec-gnb-2.c
> @@ -20,7 +20,7 @@ do_vec_gnb (vector unsigned __int128 source, int stride)
> case 5:
> return vec_gnb (source, 1); /* { dg-error "between 2 and 7" } */
> case 6:
> - return vec_gnb (source, stride); /* { dg-error "unsigned literal" } */
> + return vec_gnb (source, stride); /* { dg-error "literal" } */
> case 7:
> return vec_gnb (source, 7);
Terse :-) I think it will work fine though.
Segher
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] rs6000: Test case adjustments for new builtins
2021-11-17 12:44 ` Segher Boessenkool
@ 2021-11-17 13:52 ` Bill Schmidt
2021-11-17 18:44 ` Segher Boessenkool
0 siblings, 1 reply; 4+ messages in thread
From: Bill Schmidt @ 2021-11-17 13:52 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches, David Edelsohn
On 11/17/21 6:44 AM, Segher Boessenkool wrote:
> Hi!
>
> On Tue, Nov 16, 2021 at 02:26:22PM -0600, Bill Schmidt wrote:
>> Hi! I recently submitted [1] to make adjustments to test cases for the new builtins
>> support, mostly due to error messages changing for consistency. Thanks for the
>> previous review. I've reviewed the reasons for the changes and removed unrelated
>> changes as requested.
> And the results are? This is much easier to write up, and to review, if
> you split the patch into pieces with one theme each. If you do that
> right then most reviews will be rubber-stamping, and some might require
> some thought (and some may even get objections). The way things are it
> is a puzzle hunt to review this.
Sorry! I thought I was addressing the issues that came up last time. I didn't
intend for this to be difficult. I will break the patch up going forward.
>
>> - For fold-vect-splat-floatdouble.c and fold-vec-splat-longlong.c, the existing
>> test cases have some bad tests in them (checking two bits when only one bit
>> is meaningful). The new builtin support catches this but the old support did
>> not. Removing those bad cases changes some of the scan-assembler-times expected
>> values.
> Do this is a separate patch then, independent of the series? With this
> explanation in the commit message. This is pre-approved.
OK, will do.
>
>> - For int_128bit-runnable.c, I chose not to do gimple folding on the 128-bit
>> comparison operations in the new implementation, because doing so results in
>> bad code that splits things into two 64-bit values. That needs separate
>> attention; but the point here is, when I did that, I started generating
>> more of the vcmpequq, vcmpgtsq, and vcmpgtuq instructions.
> And you now get worse code (albeit in some cases no longer invalid)?
No, sorry that this wasn't more clear. The "old" builtins code performs
gimple folding on 128-bit compares. This results in correct but very
inefficient code. The "new" builtins code has removed the gimple folding
for 128-bit compares. This results in directly generating vcmpequq and
friends, which is the efficient code we're looking for. This test case
then needs modification to show we're doing better. I'll submit this
separately.
>
>
>> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c
>> @@ -14,7 +14,7 @@ get_exponent (double *p)
>> {
>> double source = *p;
>>
>> - return scalar_extract_exp (source); /* { dg-error "'__builtin_vec_scalar_extract_exp' is not supported in this compiler configuration" } */
>> + return scalar_extract_exp (source); /* { dg-error "'__builtin_vsx_scalar_extract_exp' requires the" } */
>> }
> The testcase uses __builtin_vec_scalar_extract_exp, so this is not okay.
Sorry, this is a case of my bad eyesight not identifying this had changed.
As with the test case (cmpb-3.c) in the 32-bit patch, this error message
isn't all that the user sees. There is also a "note" diagnostic that ties
the generic overload name to the specific underlying builtin name so that
confusion is avoided. I'll just submit these separately with a full
explanation.
Same applies to the similar cases below.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-2.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-2.c
>> @@ -12,5 +12,5 @@ get_significand (double *p)
>> {
>> double source = *p;
>>
>> - return __builtin_vec_scalar_extract_sig (source); /* { dg-error "'__builtin_vec_scalar_extract_sig' is not supported in this compiler configuration" } */
>> + return __builtin_vec_scalar_extract_sig (source); /* { dg-error "'__builtin_vsx_scalar_extract_sig' requires the" } */
>> }
> This not either.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-2.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-2.c
>> @@ -16,5 +16,5 @@ insert_exponent (unsigned long long int *significand_p,
>> unsigned long long int significand = *significand_p;
>> unsigned long long int exponent = *exponent_p;
>>
>> - return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vec_scalar_insert_exp' is not supported in this compiler configuration" } */
>> + return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vsx_scalar_insert_exp' requires the" } */
> Or this.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-5.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-5.c
>> @@ -16,5 +16,5 @@ insert_exponent (double *significand_p,
>> double significand = *significand_p;
>> unsigned long long int exponent = *exponent_p;
>>
>> - return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vec_scalar_insert_exp' is not supported in this compiler configuration" } */
>> + return scalar_insert_exp (significand, exponent); /* { dg-error "'__builtin_vsx_scalar_insert_exp_dp' requires the" } */
>> }
> Etc.
>
> It is not okay to blindly adjust the testcases to accept what the new
> code does. This is a regression. It is okay to have it regressed for a
> while. It is also okay to xfail things, if there is no expectation it
> can be fixed before the next release (or some other suitably big time
> frame, this isn't an exact science).
This isn't really a regression, as I'll describe with each patch.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-2.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-2.c
>> @@ -10,5 +10,5 @@ test_neg (float *p)
>> {
>> float source = *p;
>>
>> - return __builtin_vec_scalar_test_neg_sp (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_sp' requires" } */
>> + return __builtin_vec_scalar_test_neg (source); /* { dg-error "'__builtin_vsx_scalar_test_neg_sp' requires" } */
>> }
> This one is very curious. You change the test to use a more generic
> builtin name, presumably because the (undocumented) more specific name
> is no longer allowed, but the error message still uses that name?
I need to review this one. There were some deprecated interfaces involved here.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c
>> @@ -10,5 +10,5 @@
>> int
>> test_byte_in_set (unsigned char b, unsigned long long set_members)
>> {
>> - return __builtin_byte_in_set (b, set_members); /* { dg-warning "implicit declaration of function" } */
>> + return __builtin_byte_in_set (b, set_members); /* { dg-error "'__builtin_scalar_byte_in_set' requires the" } */
>> }
> Huh. How can the old warning ever have fired? Was the builtin not
> declared on 32-bit before? Ouch.
I'll remind myself what changed here, but yes, that's what it looks like --
an inadvertent problem with the old logic for 32-bit.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/cmpb-2.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/cmpb-2.c
>> @@ -8,7 +8,7 @@ void abort ();
>> unsigned long long int
>> do_compare (unsigned long long int a, unsigned long long int b)
>> {
>> - return __builtin_cmpb (a, b); /* { dg-warning "implicit declaration of function '__builtin_cmpb'" } */
>> + return __builtin_cmpb (a, b); /* { dg-error "'__builtin_p6_cmpb' requires the '-mcpu=power6' option" } */
>> }
> We talked about this one before already.
Yes, although it was for cmpb-3.c in the 32-bit patch. I explained what's
going on there with the "note" diagnostic.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c
>> @@ -5,21 +5,21 @@
>>
>> void use_builtins_d (__vector unsigned long long *p, __vector unsigned long long *q, __vector unsigned long long *r, __vector unsigned long long *s)
>> {
>> - p[0] = __builtin_crypto_vcipher (q[0], r[0]); /* { dg-error "'__builtin_crypto_vcipher' is not supported with the current options" } */
>> - p[1] = __builtin_crypto_vcipherlast (q[1], r[1]); /* { dg-error "'__builtin_crypto_vcipherlast' is not supported with the current options" } */
>> - p[2] = __builtin_crypto_vncipher (q[2], r[2]); /* { dg-error "'__builtin_crypto_vncipher' is not supported with the current options" } */
>> - p[3] = __builtin_crypto_vncipherlast (q[3], r[3]); /* { dg-error "'__builtin_crypto_vncipherlast' is not supported with the current options" } */
>> + p[0] = __builtin_crypto_vcipher (q[0], r[0]); /* { dg-error "'__builtin_crypto_vcipher' requires the '-mcrypto' option" } */
>> + p[1] = __builtin_crypto_vcipherlast (q[1], r[1]); /* { dg-error "'__builtin_crypto_vcipherlast' requires the '-mcrypto' option" } */
>> + p[2] = __builtin_crypto_vncipher (q[2], r[2]); /* { dg-error "'__builtin_crypto_vncipher' requires the '-mcrypto' option" } */
>> + p[3] = __builtin_crypto_vncipherlast (q[3], r[3]); /* { dg-error "'__builtin_crypto_vncipherlast' requires the '-mcrypto' option" } */
>> p[4] = __builtin_crypto_vpermxor (q[4], r[4], s[4]);
>> p[5] = __builtin_crypto_vpmsumd (q[5], r[5]);
>> - p[6] = __builtin_crypto_vshasigmad (q[6], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmad' is not supported with the current options" } */
>> - p[7] = __builtin_crypto_vsbox (q[7]); /* { dg-error "'__builtin_crypto_vsbox' is not supported with the current options" } */
>> + p[6] = __builtin_crypto_vshasigmad (q[6], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmad' requires the '-mcrypto' option" } */
>> + p[7] = __builtin_crypto_vsbox (q[7]); /* { dg-error "'__builtin_crypto_vsbox' requires the '-mcrypto' option" } */
>> }
>>
>> void use_builtins_w (__vector unsigned int *p, __vector unsigned int *q, __vector unsigned int *r, __vector unsigned int *s)
>> {
>> p[0] = __builtin_crypto_vpermxor (q[0], r[0], s[0]);
>> p[1] = __builtin_crypto_vpmsumw (q[1], r[1]);
>> - p[2] = __builtin_crypto_vshasigmaw (q[2], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmaw' is not supported with the current options" } */
>> + p[2] = __builtin_crypto_vshasigmaw (q[2], 1, 15); /* { dg-error "'__builtin_crypto_vshasigmaw' requires the '-mcrypto' option" } */
>> }
> This one is fine.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-floatdouble.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-floatdouble.c
>> @@ -18,7 +18,7 @@ vector float test_fc ()
>> vector double testd_00 (vector double x) { return vec_splat (x, 0b00000); }
>> vector double testd_01 (vector double x) { return vec_splat (x, 0b00001); }
>> vector double test_dc ()
>> -{ const vector double y = { 3.0, 5.0 }; return vec_splat (y, 0b00010); }
>> +{ const vector double y = { 3.0, 5.0 }; return vec_splat (y, 0b00001); }
>>
>> /* If the source vector is a known constant, we will generate a load or possibly
>> XXSPLTIW. */
>> @@ -28,5 +28,5 @@ vector double test_dc ()
>> /* { dg-final { scan-assembler-times {\mvspltw\M|\mxxspltw\M} 3 } } */
>>
>> /* For double types, we will generate xxpermdi instructions. */
>> -/* { dg-final { scan-assembler-times "xxpermdi" 3 } } */
>> +/* { dg-final { scan-assembler-times "xxpermdi" 2 } } */
> This is okay as a separate patch, with proper commit message.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c
>> @@ -9,23 +9,19 @@
>>
>> vector bool long long testb_00 (vector bool long long x) { return vec_splat (x, 0b00000); }
>> vector bool long long testb_01 (vector bool long long x) { return vec_splat (x, 0b00001); }
>> -vector bool long long testb_02 (vector bool long long x) { return vec_splat (x, 0b00010); }
>>
>> vector signed long long tests_00 (vector signed long long x) { return vec_splat (x, 0b00000); }
>> vector signed long long tests_01 (vector signed long long x) { return vec_splat (x, 0b00001); }
>> -vector signed long long tests_02 (vector signed long long x) { return vec_splat (x, 0b00010); }
>>
>> vector unsigned long long testu_00 (vector unsigned long long x) { return vec_splat (x, 0b00000); }
>> vector unsigned long long testu_01 (vector unsigned long long x) { return vec_splat (x, 0b00001); }
>> -vector unsigned long long testu_02 (vector unsigned long long x) { return vec_splat (x, 0b00010); }
>>
>> /* Similar test as above, but the source vector is a known constant. */
>> -vector bool long long test_bll () { const vector bool long long y = {12, 23}; return vec_splat (y, 0b00010); }
>> -vector signed long long test_sll () { const vector signed long long y = {34, 45}; return vec_splat (y, 0b00010); }
>> -vector unsigned long long test_ull () { const vector unsigned long long y = {56, 67}; return vec_splat (y, 0b00010); }
>> +vector bool long long test_bll () { const vector bool long long y = {12, 23}; return vec_splat (y, 0b00001); }
>> +vector signed long long test_sll () { const vector signed long long y = {34, 45}; return vec_splat (y, 0b00001); }
>>
>> /* Assorted load instructions for the initialization with known constants. */
>> -/* { dg-final { scan-assembler-times {\mlvx\M|\mlxvd2x\M|\mlxv\M|\mplxv\M} 3 } } */
>> +/* { dg-final { scan-assembler-times {\mlvx\M|\mlxvd2x\M|\mlxv\M|\mplxv\M|\mxxspltib\M} 2 } } */
> Ditto.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-misc-invalid.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-misc-invalid.c
>> @@ -10,24 +10,24 @@
>> vector signed short
>> testss_1 (unsigned int ui)
>> {
>> - return vec_splat_s16 (ui);/* { dg-error "argument 1 must be a 5-bit signed literal" } */
>> + return vec_splat_s16 (ui);/* { dg-error "argument 1 must be a literal between -16 and 15, inclusive" } */
>> }
> All such things are fine of course.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
>> @@ -11,9 +11,9 @@
>> /* { dg-final { scan-assembler-times {\mvrlq\M} 2 } } */
>> /* { dg-final { scan-assembler-times {\mvrlqnm\M} 2 } } */
>> /* { dg-final { scan-assembler-times {\mvrlqmi\M} 2 } } */
>> -/* { dg-final { scan-assembler-times {\mvcmpequq\M} 16 } } */
>> -/* { dg-final { scan-assembler-times {\mvcmpgtsq\M} 16 } } */
>> -/* { dg-final { scan-assembler-times {\mvcmpgtuq\M} 16 } } */
>> +/* { dg-final { scan-assembler-times {\mvcmpequq\M} 24 } } */
>> +/* { dg-final { scan-assembler-times {\mvcmpgtsq\M} 26 } } */
>> +/* { dg-final { scan-assembler-times {\mvcmpgtuq\M} 26 } } */
>> /* { dg-final { scan-assembler-times {\mvmuloud\M} 1 } } */
>> /* { dg-final { scan-assembler-times {\mvmulesd\M} 1 } } */
>> /* { dg-final { scan-assembler-times {\mvmulosd\M} 1 } } */
> This either needs more explanation (and be a separate patch), or it is
> just wrong :-(
This is the gimple-fold issue discussed at the top. Will submit
separately.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
>> @@ -10,6 +10,6 @@ main ()
>> int mask;
>>
>> /* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */
>> - res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error {argument 3 must be in the range \[0, 15\]} } */
>> + res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error {argument 3 must be a 4-bit unsigned literal} } */
>> return 0;
>> }
> Hrm, make this say "must be a literal between 0 and 15, inclusive" like
> the other errors?
The "n-bit unsigned literal" is the usual case. I'll provide more explanation
in the separate patch.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
>> @@ -8,13 +8,13 @@ int main ()
>> int arguments. The builtins __builtin_set_fpscr_rn() also supports a
>> variable as an argument but can't test variable value at compile time. */
>>
>> - __builtin_mtfsb0(-1); /* { dg-error "Argument must be a constant between 0 and 31" } */
>> - __builtin_mtfsb0(32); /* { dg-error "Argument must be a constant between 0 and 31" } */
>> + __builtin_mtfsb0(-1); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
>> + __builtin_mtfsb0(32); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
>>
>> - __builtin_mtfsb1(-1); /* { dg-error "Argument must be a constant between 0 and 31" } */
>> - __builtin_mtfsb1(32); /* { dg-error "Argument must be a constant between 0 and 31" } */
>> + __builtin_mtfsb1(-1); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
>> + __builtin_mtfsb1(32); /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
>>
>> - __builtin_set_fpscr_rn(-1); /* { dg-error "Argument must be a value between 0 and 3" } */
>> - __builtin_set_fpscr_rn(4); /* { dg-error "Argument must be a value between 0 and 3" } */
>> + __builtin_set_fpscr_rn(-1); /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */
>> + __builtin_set_fpscr_rn(4); /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */
>> }
> This regressed as well.
I don't think it's a regression, just a better error message. Will add explanation
and submit separately.
>
>> --- a/gcc/testsuite/gcc.target/powerpc/vec-gnb-2.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/vec-gnb-2.c
>> @@ -20,7 +20,7 @@ do_vec_gnb (vector unsigned __int128 source, int stride)
>> case 5:
>> return vec_gnb (source, 1); /* { dg-error "between 2 and 7" } */
>> case 6:
>> - return vec_gnb (source, stride); /* { dg-error "unsigned literal" } */
>> + return vec_gnb (source, stride); /* { dg-error "literal" } */
>> case 7:
>> return vec_gnb (source, 7);
> Terse :-) I think it will work fine though.
Again, I'm sorry this was difficult to review. Originally I thought it would be easiest
to keep all these together, but that clearly wasn't helpful. I'll work on breaking
this up.
Thanks for the review!
Bill
>
>
> Segher
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] rs6000: Test case adjustments for new builtins
2021-11-17 13:52 ` Bill Schmidt
@ 2021-11-17 18:44 ` Segher Boessenkool
0 siblings, 0 replies; 4+ messages in thread
From: Segher Boessenkool @ 2021-11-17 18:44 UTC (permalink / raw)
To: Bill Schmidt; +Cc: GCC Patches, David Edelsohn
On Wed, Nov 17, 2021 at 07:52:38AM -0600, Bill Schmidt wrote:
> >> - For int_128bit-runnable.c, I chose not to do gimple folding on the 128-bit
> >> comparison operations in the new implementation, because doing so results in
> >> bad code that splits things into two 64-bit values. That needs separate
> >> attention; but the point here is, when I did that, I started generating
> >> more of the vcmpequq, vcmpgtsq, and vcmpgtuq instructions.
> > And you now get worse code (albeit in some cases no longer invalid)?
>
> No, sorry that this wasn't more clear. The "old" builtins code performs
> gimple folding on 128-bit compares. This results in correct but very
> inefficient code. The "new" builtins code has removed the gimple folding
> for 128-bit compares. This results in directly generating vcmpequq and
> friends, which is the efficient code we're looking for. This test case
> then needs modification to show we're doing better. I'll submit this
> separately.
Hrm. Folding should always be a good thing to do; and folding should
never split an operation on a 128-bit datum into two operations on
64-bit things. That kind of optimisation cannot be sanely done on
Gimple level: the abstractions are not close enough to the hardware for
that, and the instruction stream is not close at all to what the
eventual machine insns will be. We have an RTL pass that does this
("subreg"), it runs almost immediately after expand (and two more
times, even again after the split pass).
So there is a generic bug that you counteract with a target bug :-(
> >> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c
> >> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-2.c
> >> @@ -14,7 +14,7 @@ get_exponent (double *p)
> >> {
> >> double source = *p;
> >>
> >> - return scalar_extract_exp (source); /* { dg-error "'__builtin_vec_scalar_extract_exp' is not supported in this compiler configuration" } */
> >> + return scalar_extract_exp (source); /* { dg-error "'__builtin_vsx_scalar_extract_exp' requires the" } */
> >> }
> > The testcase uses __builtin_vec_scalar_extract_exp, so this is not okay.
>
> Sorry, this is a case of my bad eyesight not identifying this had changed.
> As with the test case (cmpb-3.c) in the 32-bit patch, this error message
> isn't all that the user sees. There is also a "note" diagnostic that ties
> the generic overload name to the specific underlying builtin name so that
> confusion is avoided. I'll just submit these separately with a full
> explanation.
Can't you go just two inches further and report the actual builtin used
by the user (which even is documented!), and not cause any confusion?
> > It is not okay to blindly adjust the testcases to accept what the new
> > code does. This is a regression. It is okay to have it regressed for a
> > while. It is also okay to xfail things, if there is no expectation it
> > can be fixed before the next release (or some other suitably big time
> > frame, this isn't an exact science).
>
> This isn't really a regression, as I'll describe with each patch.
Looking forward to it :-)
> >> --- a/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c
> >> +++ b/gcc/testsuite/gcc.target/powerpc/byte-in-set-2.c
> >> @@ -10,5 +10,5 @@
> >> int
> >> test_byte_in_set (unsigned char b, unsigned long long set_members)
> >> {
> >> - return __builtin_byte_in_set (b, set_members); /* { dg-warning "implicit declaration of function" } */
> >> + return __builtin_byte_in_set (b, set_members); /* { dg-error "'__builtin_scalar_byte_in_set' requires the" } */
> >> }
> > Huh. How can the old warning ever have fired? Was the builtin not
> > declared on 32-bit before? Ouch.
>
> I'll remind myself what changed here, but yes, that's what it looks like --
> an inadvertent problem with the old logic for 32-bit.
In general it is better to always have all builtins (and other
interfaces) declared internally, so that you can give much better error
messages (and so that you get errors if there are conflicts, etc.)
There can be exceptions, but this is not a case like that :-) (So your
change is great :-) )
> >> --- a/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
> >> +++ b/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
> >> @@ -10,6 +10,6 @@ main ()
> >> int mask;
> >>
> >> /* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */
> >> - res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error {argument 3 must be in the range \[0, 15\]} } */
> >> + res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error {argument 3 must be a 4-bit unsigned literal} } */
> >> return 0;
> >> }
> > Hrm, make this say "must be a literal between 0 and 15, inclusive" like
> > the other errors?
>
> The "n-bit unsigned literal" is the usual case. I'll provide more explanation
> in the separate patch.
We should use the same formulation always. I like the more verbose more
exact less confusing and even *correct* formulation :-)
> Again, I'm sorry this was difficult to review. Originally I thought it would be easiest
> to keep all these together, but that clearly wasn't helpful. I'll work on breaking
> this up.
In general it is best to keep testcase changes together with the patch
that necessitates them. You cannot use this now; this is one of the
reasons why it is much better to do the changes step by step (where
every change is immediately engaged!) This is more work up front, but
it may well be less work in total. It certainly is less frustrating :-)
Segher
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-17 18:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16 20:26 [PATCH v2] rs6000: Test case adjustments for new builtins Bill Schmidt
2021-11-17 12:44 ` Segher Boessenkool
2021-11-17 13:52 ` Bill Schmidt
2021-11-17 18:44 ` Segher Boessenkool
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).