Ping Patch: PR target/24879: SSE3 intrinsic bug

[H. J. Lu]

On Tue, May 02, 2006 at 10:31:52PM -0700, Ian Lance Taylor wrote:
> "H. J. Lu" <hjl@lucon.org> writes:
> 
> > Most, if not all, of __builtin_ia32_xxx for intrinsics map to
> > instructions directly and users are supposed to use intrinsics, not
> > builtins directly. I have no strong opinions on this. I can certainly
> > change __builtin_ia32_mwait and __builtin_ia32_monitor not to follow
> > it.
> 
> As far as I can see, we gain nothing from having __builtin_ia32_mwait
> and __builtin_ia32_monitor be different in 32-bit mode and 64-bit
> mode.  The code becomes more complex to no purpose.  So why do it?
> 

Here is the updated patch.


H.J.
----
gcc/

2006-05-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/24879
	* config/i386/i386.c (ix86_init_mmx_sse_builtins): Add
	replace void_ftype_pcvoid_unsigned_unsigned with
	void_ftype_unsigned_unsigned_unsigned.

	* config/i386/pmmintrin.h (_mm_monitor): Remove macro. Use
	inline function.
	(_mm_mwait): Likewise.

	* config/i386/sse.md (sse3_mwait): Replace "mwait\t%0, %1" with
	"mwait".
	(sse3_monitor): Replace "monitor\t%0, %1, %2" with "monitor".

gcc/testsuite/

2006-05-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/24879
	* gcc.target/i386/monitor.c: New file.

--- gcc/config/i386/i386.c.64bit	2006-05-03 06:22:19.000000000 -0700
+++ gcc/config/i386/i386.c	2006-05-03 06:39:59.000000000 -0700
@@ -14915,8 +14915,8 @@ ix86_init_mmx_sse_builtins (void)
   tree void_ftype_unsigned_unsigned
     = build_function_type_list (void_type_node, unsigned_type_node,
 				unsigned_type_node, NULL_TREE);
-  tree void_ftype_pcvoid_unsigned_unsigned
-    = build_function_type_list (void_type_node, const_ptr_type_node,
+  tree void_ftype_unsigned_unsigned_unsigned
+    = build_function_type_list (void_type_node, unsigned_type_node,
 				unsigned_type_node, unsigned_type_node,
 				NULL_TREE);
   tree unsigned_ftype_void
@@ -15440,7 +15440,7 @@ ix86_init_mmx_sse_builtins (void)
 
   /* Prescott New Instructions.  */
   def_builtin (MASK_SSE3, "__builtin_ia32_monitor",
-	       void_ftype_pcvoid_unsigned_unsigned,
+	       void_ftype_unsigned_unsigned_unsigned,
 	       IX86_BUILTIN_MONITOR);
   def_builtin (MASK_SSE3, "__builtin_ia32_mwait",
 	       void_ftype_unsigned_unsigned,
--- gcc/config/i386/pmmintrin.h.64bit	2006-05-03 06:20:37.000000000 -0700
+++ gcc/config/i386/pmmintrin.h	2006-05-03 06:51:52.000000000 -0700
@@ -1,4 +1,4 @@
-/* Copyright (C) 2003, 2004, 2005 Free Software Foundation, Inc.
+/* Copyright (C) 2003, 2004, 2005, 2006 Free Software Foundation, Inc.
 
    This file is part of GCC.
 
@@ -25,7 +25,7 @@
    Public License.  */
 
 /* Implemented from the specification included in the Intel C++ Compiler
-   User Guide and Reference, version 8.0.  */
+   User Guide and Reference, version 9.0.  */
 
 #ifndef _PMMINTRIN_H_INCLUDED
 #define _PMMINTRIN_H_INCLUDED
@@ -110,11 +110,14 @@ _mm_lddqu_si128 (__m128i const *__P)
   return (__m128i) __builtin_ia32_lddqu ((char const *)__P);
 }
 
-#if 0
 static __inline void __attribute__((__always_inline__))
 _mm_monitor (void const * __P, unsigned int __E, unsigned int __H)
 {
-  __builtin_ia32_monitor (__P, __E, __H);
+#ifdef __x86_64__
+  __builtin_ia32_monitor ((unsigned int) (unsigned long) __P, __E, __H);
+#else
+  __builtin_ia32_monitor ((unsigned int) __P, __E, __H);
+#endif
 }
 
 static __inline void __attribute__((__always_inline__))
@@ -122,10 +125,6 @@ _mm_mwait (unsigned int __E, unsigned in
 {
   __builtin_ia32_mwait (__E, __H);
 }
-#else
-#define _mm_monitor(P, E, H)	__builtin_ia32_monitor ((P), (E), (H))
-#define _mm_mwait(E, H)		__builtin_ia32_mwait ((E), (H))
-#endif
 
 #endif /* __SSE3__ */
 
--- gcc/config/i386/sse.md.64bit	2006-05-03 06:20:37.000000000 -0700
+++ gcc/config/i386/sse.md	2006-05-03 06:29:35.000000000 -0700
@@ -3972,7 +3972,10 @@
 		     (match_operand:SI 1 "register_operand" "c")]
 		    UNSPECV_MWAIT)]
   "TARGET_SSE3"
-  "mwait\t%0, %1"
+;; 64bit version is "mwait %rax,%rcx". But only lower 32bits are
+;; used.  Since 32bit register operands are implicitly zero extended to
+;; 64bit, we only need to set up 32bit registers.
+  "mwait"
   [(set_attr "length" "3")])
 
 (define_insn "sse3_monitor"
@@ -3981,7 +3984,10 @@
 		     (match_operand:SI 2 "register_operand" "d")]
 		    UNSPECV_MONITOR)]
   "TARGET_SSE3"
-  "monitor\t%0, %1, %2"
+;; 64bit version is "monitor %rax,%rcx,%rdx". But only lower 32bits are
+;; used.  Since 32bit register operands are implicitly zero extended to
+;; 64bit, we only need to set up 32bit registers.
+  "monitor"
   [(set_attr "length" "3")])
 
 ;; MNI
--- gcc/testsuite/gcc.target/i386/monitor.c.64bit	2006-05-03 06:20:37.000000000 -0700
+++ gcc/testsuite/gcc.target/i386/monitor.c	2006-05-03 06:22:45.000000000 -0700
@@ -0,0 +1,27 @@
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -msse3" } */
+
+/* Verify that they work in both 32bit and 64bit.  */
+
+#include <pmmintrin.h>
+
+void
+foo (char *p, int x, int y, int z)
+{
+   _mm_monitor (p, y, x);
+   _mm_mwait (z, y);
+}
+
+void
+bar (char *p, long x, long y, long z)
+{
+   _mm_monitor (p, y, x);
+   _mm_mwait (z, y);
+}
+
+void
+foo1 (char *p)
+{
+   _mm_monitor (p, 0, 0);
+   _mm_mwait (0, 0);
+}