Re: PATCH: PR target/32000: x86 backend uses aligned load on unaligned memory

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Mar 31, 2008 at 12:50:51PM +0800, Zuxy Meng wrote:
> Hi,
>
> "H.J. Lu" <hjl.tools@gmail.com> ååæææé:20080330212721.GA7821@lucon.org...
>> Hi,
>>
>> movti may generate SSE vector move for TDmode. When TDmode is
>> aligned at 1 byte, movti generates aligned load on unaligned
>> memory.  This patch adds a new predicate and uses it to check
>> misaligned operand to generate unaligned move if needed.
>>
>>
>> ----
>> 2008-03-30  H.J. Lu  <hongjiu.lu@intel.com>
>>
>> PR target/32000
>> * config/i386/i386.md (*movti_internal): Emit unaligned SSE
>> load/store if memory is unaligned.
>> (*movti_rex64): Likewise.
>>
>> * config/i386/predicates.md (misaligned_operand): New.
>>
>> 2008-03-30  H.J. Lu  <hongjiu.lu@intel.com>
>>
>> PR target/32000
>> * gcc.target/i386/pr32000-1.c: New.
>> * gcc.target/i386/pr32000-2.c: Likewise.
>> * gcc.target/i386/pr32000-3.c: Likewise.
>> * gcc.target/i386/pr32000-4.c: Likewise.
>> * gcc.target/i386/pr32000-5.c: Likewise.
>> * gcc.target/i386/pr32000-6.c: Likewise.
>>
>
>
> No cases for movapd and movupd?

SSE modes do have a similar bug:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35767

I will submit a separate patch for PR 35767.

Here is the updated patch with only one simplied testcase.  pr32000-2.c
belongs to PR 35767 and the other testcases aren't needed when
PR 35767 is fixed.



H.J.
----
2008-03-30  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/32000
	* config/i386/i386.md (*movti_internal): Emit unaligned SSE
	load/store if memory is unaligned.
	(*movti_rex64): Likewise.

	* config/i386/predicates.md (misaligned_operand): New.

2008-03-30  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/32000
	* gcc.target/i386/pr32000-1.c: New.

--- gcc/config/i386/i386.md.td	2008-03-30 10:07:59.000000000 -0700
+++ gcc/config/i386/i386.md	2008-03-30 10:07:59.000000000 -0700
@@ -2387,10 +2387,23 @@
 	return "pxor\t%0, %0";
     case 1:
     case 2:
-      if (get_attr_mode (insn) == MODE_V4SF)
-	return "movaps\t{%1, %0|%0, %1}";
+      /* TDmode values are passed as TImode on the stack.  Moving them
+	 to stack may result in unaligned memory access.  */
+      if (misaligned_operand (operands[0], TImode)
+	  || misaligned_operand (operands[1], TImode))
+	{ 
+	  if (get_attr_mode (insn) == MODE_V4SF)
+	    return "movups\t{%1, %0|%0, %1}";
+	 else
+	   return "movdqu\t{%1, %0|%0, %1}";
+	}
       else
-	return "movdqa\t{%1, %0|%0, %1}";
+	{ 
+	  if (get_attr_mode (insn) == MODE_V4SF)
+	    return "movaps\t{%1, %0|%0, %1}";
+	 else
+	   return "movdqa\t{%1, %0|%0, %1}";
+	}
     default:
       gcc_unreachable ();
     }
@@ -2424,10 +2437,23 @@
 	return "pxor\t%0, %0";
     case 3:
     case 4:
-      if (get_attr_mode (insn) == MODE_V4SF)
-	return "movaps\t{%1, %0|%0, %1}";
+      /* TDmode values are passed as TImode on the stack.  Moving them
+	 to stack may result in unaligned memory access.  */
+      if (misaligned_operand (operands[0], TImode)
+	  || misaligned_operand (operands[1], TImode))
+	{ 
+	  if (get_attr_mode (insn) == MODE_V4SF)
+	    return "movups\t{%1, %0|%0, %1}";
+	 else
+	   return "movdqu\t{%1, %0|%0, %1}";
+	}
       else
-	return "movdqa\t{%1, %0|%0, %1}";
+	{ 
+	  if (get_attr_mode (insn) == MODE_V4SF)
+	    return "movaps\t{%1, %0|%0, %1}";
+	 else
+	   return "movdqa\t{%1, %0|%0, %1}";
+	}
     default:
       gcc_unreachable ();
     }
--- gcc/config/i386/predicates.md.td	2008-03-30 08:33:33.000000000 -0700
+++ gcc/config/i386/predicates.md	2008-03-30 10:07:59.000000000 -0700
@@ -1043,3 +1043,8 @@
 
 (define_predicate "absneg_operator"
   (match_code "abs,neg"))
+
+;; Return 1 if OP is misaligned memory operand
+(define_predicate "misaligned_operand"
+  (and (match_code "mem")
+       (match_test "MEM_ALIGN (op) < GET_MODE_ALIGNMENT (mode)")))
--- gcc/testsuite/gcc.target/i386/pr32000-1.c.td	2008-03-30 10:07:59.000000000 -0700
+++ gcc/testsuite/gcc.target/i386/pr32000-1.c	2008-03-30 21:26:10.000000000 -0700
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+/* { dg-require-effective-target dfp } */
+/* { dg-options "-O -msse2 -std=gnu99" } */
+
+#include "sse2-check.h"
+
+typedef struct { _Decimal128 f __attribute__((packed)); } packed;
+
+_Decimal128 __attribute__((noinline))
+foo (_Decimal128 a1, _Decimal128 a2, _Decimal128 a3, _Decimal128 a4,
+     _Decimal128 a5, _Decimal128 a6, _Decimal128 a7, _Decimal128 a8,
+     int b1, int b2, int b3, int b4, int b5, int b6, int b7, packed y)
+{
+  return y.f;
+}
+
+void
+sse2_test (void)
+{
+  packed x;
+  _Decimal128 y = -1;
+  x.f = y;
+  y = foo (0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3, 4, 5, 6, -1, x);
+  if (__builtin_memcmp (&y, &x.f, sizeof (y)))
+    abort ();
+}