Re: PATCH: Add LWP support for upcoming AMD Orochi processor.

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Nov 05, 2009 at 10:32:07AM +0100, Jakub Jelinek wrote:
> On Thu, Oct 08, 2009 at 07:43:43PM -0500, Harsha Jagasia wrote:
> > 	* doc/invoke.texi (-mlwp): Add documentation.
> > 	* doc/extend.texi (x86 intrinsics): Add LWP intrinsics.
> ...
> 
> I see this got committed now, but testsuite hasn't been adjusted for it.
> Please at least change gcc.dg/target/i386/sse-{12,13,14,22,23}.c, so that
> they mention also testing lwpintrin.h (search for xopintrin.h), where
> compiled with -mxop they are also compiled with -mlwp, etc.
> BTW, sse-23.c mentions xopintrin.h just in comments, but doesn't actually
> include xopintrin.h nor x86intrin.h, so it probably needs tweaking also for
> xop.  Also, if possible, please add some lwp testcases.

As the following patch proves, lwpintrin.h and the whole lwp support is
quite severely broken.

1) llwpcb* - the builtins are declared void __builtin_ia32_llwpcb* (void),
but lwpintrin.h expects them to take void * argument.  If I understand right, the
insn in reality has 3 address sizes to support 16-bit/32-bit/64-bit code,
I fail to see why we'd need 3 different intrinsics, instead of just one and
one builtin that takes void * address and uses the insn matching Pmode.
Furthermore, as the insn has no output, I believe you have to use UNSPECV
instead of UNSPEC.

2) slwpcb* - similarly, the builtins are VOID_FTYPE_VOID, when it is
expected to be void *__builtin_ia32_slwpcb* (void).  Again, I fail to see
why 3 intrinsics are needed, just one would be enough.  In i386.md they have
wrong patterns, as they set the registers it should be something like:
(define_insn "lwp_slwpcb<mode>1"
  [(set (match_operand:P 0 "register_operand" "=r")
	(unspec [(const_int 0)] UNSPEC_SLWP_INTRINSIC))]
  "TARGET_LWP"
  "slwpcb\t%0"
  [(set_attr "type" "lwp")
   (set_attr "mode" "<MODE>")])

3) lwpval*
  { OPTION_MASK_ISA_LWP, CODE_FOR_lwp_lwpvalhi3, "__builtin_ia32_lwpval16", IX86_BUILTIN_LWPVAL16,  UNKNOWN,     (int) VOID_FTYPE_USHORT_UINT_USHORT },                                                                                                         
  { OPTION_MASK_ISA_LWP, CODE_FOR_lwp_lwpvalsi3, "__builtin_ia32_lwpval32", IX86_BUILTIN_LWPVAL64,  UNKNOWN,     (int) VOID_FTYPE_UINT_UINT_UINT },                                                                                                             
  { OPTION_MASK_ISA_LWP, CODE_FOR_lwp_lwpvaldi3, "__builtin_ia32_lwpval64", IX86_BUILTIN_LWPVAL64,  UNKNOWN,     (int) VOID_FTYPE_UINT64_UINT_UINT },
typo on the second line, s/IX86_BUILTIN_LWPVAL64/IX86_BUILTIN_LWPVAL32/
there.
Also, I don't think we have anything like unsigned __int64
type on Linux, guess you want to use int __attribute__((__mode__(__DI__)))
instead.

4) lwpins* is written to return char, what is the return value?
rFlags.CF value after the insn?  The insn pattern needs to be rewritten
to make it clear that it sets the (reg:CC FLAGS_REG) to some unspec
value.  i386.c has similar typo (LWPINS64 instead of LWPINS32 for
lwpins32).  And it needs to arrange for the return value to be somehow set
(define_expand that expands it to the actual lwpins insn plus setc insn?).

2009-11-05  Jakub Jelinek  <jakub@redhat.com>

	* gcc.target/i386/sse-12.c: Compile also with -mlwp.
	* gcc.target/i386/sse-13.c: Likewise.
	* gcc.target/i386/sse-14.c: Likewise.
	* gcc.target/i386/sse-23.c: Add lwp to GCC target pragma,
	include also x86intrin.h.

--- gcc/testsuite/gcc.target/i386/sse-12.c.jj	2009-11-04 18:31:22.000000000 +0100
+++ gcc/testsuite/gcc.target/i386/sse-12.c	2009-11-05 16:20:54.000000000 +0100
@@ -1,7 +1,7 @@
-/* Test that {,x,e,p,t,s,w,a,b,i}mmintrin.h, xopintrin.h, mm3dnow.h and mm_malloc.h are
-   usable with -O -std=c89 -pedantic-errors.  */
+/* Test that {,x,e,p,t,s,w,a,b,i}mmintrin.h, xopintrin.h, lwpintrin.h, mm3dnow.h
+   and mm_malloc.h are usable with -O -std=c89 -pedantic-errors.  */
 /* { dg-do compile } */
-/* { dg-options "-O -std=c89 -pedantic-errors -march=k8 -m3dnow -mavx -mfma4 -mxop -maes -mpclmul" } */
+/* { dg-options "-O -std=c89 -pedantic-errors -march=k8 -m3dnow -mavx -mfma4 -mxop -mlwp -maes -mpclmul" } */
 
 #include <x86intrin.h>
 
--- gcc/testsuite/gcc.target/i386/sse-13.c.jj	2009-11-04 18:31:22.000000000 +0100
+++ gcc/testsuite/gcc.target/i386/sse-13.c	2009-11-05 16:21:48.000000000 +0100
@@ -1,12 +1,13 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -Werror-implicit-function-declaration -march=k8 -m3dnow -mavx -mxop -maes -mpclmul" } */
+/* { dg-options "-O2 -Werror-implicit-function-declaration -march=k8 -m3dnow -mavx -mxop -mlwp -maes -mpclmul" } */
 
 #include <mm_malloc.h>
 
 /* Test that the intrinsics compile with optimization.  All of them are
-   defined as inline functions in {,x,e,p,t,s,w,a,b,i}mmintrin.h, xopintrin.h and mm3dnow.h
-   that reference the proper builtin functions.  Defining away "extern" and
-   "__inline" results in all of them being compiled as proper functions.  */
+   defined as inline functions in {,x,e,p,t,s,w,a,b,i}mmintrin.h, xopintrin.h,
+   lwpintrin.h and mm3dnow.h that reference the proper builtin functions.
+   Defining away "extern" and "__inline" results in all of them being compiled
+   as proper functions.  */
 
 #define extern
 #define __inline
--- gcc/testsuite/gcc.target/i386/sse-14.c.jj	2009-11-04 18:31:22.000000000 +0100
+++ gcc/testsuite/gcc.target/i386/sse-14.c	2009-11-05 16:26:39.000000000 +0100
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O0 -Werror-implicit-function-declaration -march=k8 -m3dnow -mavx -mxop -msse4a -maes -mpclmul" } */
+/* { dg-options "-O0 -Werror-implicit-function-declaration -march=k8 -m3dnow -mavx -mxop -mlwp -msse4a -maes -mpclmul" } */
 
 #include <mm_malloc.h>
 
--- gcc/testsuite/gcc.target/i386/sse-23.c.jj	2009-11-04 18:31:22.000000000 +0100
+++ gcc/testsuite/gcc.target/i386/sse-23.c	2009-11-05 16:30:40.000000000 +0100
@@ -4,9 +4,10 @@
 #include <mm_malloc.h>
 
 /* Test that the intrinsics compile with optimization.  All of them are
-   defined as inline functions in {,x,e,p,t,s,w,a}mmintrin.h, xopintrin.h and mm3dnow.h
-   that reference the proper builtin functions.  Defining away "extern" and
-   "__inline" results in all of them being compiled as proper functions.  */
+   defined as inline functions in {,x,e,p,t,s,w,a}mmintrin.h, xopintrin.h,
+   lwpintrin.h and mm3dnow.h that reference the proper builtin functions.
+   Defining away "extern" and "__inline" results in all of them being compiled
+   as proper functions.  */
 
 #define extern
 #define __inline
@@ -99,7 +100,8 @@
 #define __builtin_ia32_vprotdi(A, B) __builtin_ia32_vprotdi(A,1)
 #define __builtin_ia32_vprotqi(A, B) __builtin_ia32_vprotqi(A,1)
 
-#pragma GCC target ("3dnow,sse4,sse4a,aes,pclmul,xop")
+#pragma GCC target ("3dnow,sse4,sse4a,aes,pclmul,xop,lwp")
 #include <wmmintrin.h>
 #include <smmintrin.h>
 #include <mm3dnow.h>
+#include <x86intrin.h>


	Jakub