GCC maintainers:
Here is my respnses to the review comments by Kewen. Unfortunately,
Kewen is no longer working on GCC power.
I will submit an updated version of the patch with Kewen's suggested
changes.
Carl
On 8/9/24 3:11 AM, Kewen.Lin wrote:
Hi Carl,
on 2024/8/8 01:15, Carl Love wrote:
GCC maintainers:
The following patch adds missing test cases for the overloaded vec_perm
built-in. It also fixes and issue with printing the 128-bit values in the
DEBUG section that was noticed when adding the additional test cases.
The patch has been tested on Power 10 LE and BE with no regressions.
Please let me know if it is acceptable for mainline. Thanks.
Carl
-------------------------------------------------------------
rs6000, add testcases to the overloaded vec_perm built-in
The overloaded vec_perm built-in supports permuting signed and unsigned
vectors of char, bool char, short int, short bool, int, bool,
long long int, long long bool, int128, float and double. However, not all
of the supported arguments are included in the test cases. This patch adds
the missing test cases.
Additionally, in the 128-bit debug print statements the expected result and
the result need to be cast to unsigned long long to print correctly. The
patch makes this additional change to the print statements.
gcc/ChangeLog:
* doc/extend.texi: Fix spelling mistake in description of the
vec_sel built-in.
Add documentation of the 128-bit vec_perm instance.
gcc/testsuite/ChangeLog:
* gcc.target/powerpc/vsx-builtin-3.c: Add vec_perm test cases for
arguments of type vector signed long long int, long long bool,
bool, bool short, bool char and pixel,
vector unsigned long long int, unsigned int, unsigned short int,
unsigned char.
Cast arguments for debug prints to unsigned long long.
* gcc.target/powerpc/builtins-4-int128-runnable.c: Add vec_perm
test cases for signed and unsigned int128 arguments.
Nit: Some changelog lines have unnecessary newlines and spaces.
Fixed.
---
gcc/doc/extend.texi | 12 +-
.../powerpc/builtins-4-int128-runnable.c | 108 +++++++++++++++---
.../gcc.target/powerpc/vsx-builtin-3.c | 18 +++
3 files changed, 121 insertions(+), 17 deletions(-)
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 48b27ff9f39..bf6f4094040 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -21553,9 +21553,19 @@ vector bool __int128 vec_sel (vector bool __int128,
vector bool __int128, vector unsigned __int128);
@end smallexample
-The instance is an extension of the exiting overloaded built-in @code{vec_sel}
+The instance is an extension of the existing overloaded built-in @code{vec_sel}
that is documented in the PVIPR.
Good catch!
+@smallexample
+vector signed __int128 vec_perm (vector signed __int128,
+ vector signed __int128);
+vector unsigned __int128 vec_perm (vector unsigned __int128,
+ vector unsigned __int128);
+@end smallexample
+
+The 128-bit integer arguments for the @code{vec_perm} built-in are in addition
+to the instances that are documented in the PVIPR.
Nit: Maybe just copy the above wording for @code{vec_sel} but replaced with
@code{vec_perm} to keep them consistent.
OK, made them consistent.
<snip>
diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
index 67c93be1469..b3b76be34b9 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
@@ -39,10 +39,17 @@
#include <altivec.h>
+extern __vector long long int sll[][4];
There is a "extern __vector long long sll[][4]" below.
+extern __vector long long bool bll[][4];
extern __vector int si[][4];
+extern __vector bool int bi[][4];
Similar, having "... __vector __bool int bi[][4]" below.
extern __vector short ss[][4];
+extern __vector bool short bs[][4];
Similar, having "... __vector __bool short bs[][4]" below.
extern __vector signed char sc[][4];
+extern __vector bool char bc[][4];
Ditto.
+extern __vector pixel p[][4];
Similar, having "... __vector __pixel p[][4]" below.
extern __vector float f[][4];
+extern __vector unsigned long long int ull[][4];
As above, I think we only need "bll" and "ull" here.
Yea, looks like I didn't notice that they were previously defined. Looks
like all I really needed to add is the bll. There is as ull definition
already for __VSX__ which I think needs to be moved so it is always there.
Surprised the compiler didn't complain about multiple definitions.
Carl
