diff mbox

[i386] : Fix PR 46716, [4.4/4.5/4.6/4.7/4.8 Regression] wrong code generated with -mno-sse2 -m64

Message ID CAFULd4YF3dPz_L--bawxB9wSatVFKhXjMB-0rrqA8cNLci4cAA@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak March 2, 2012, 4:32 p.m. UTC 
Hello!

i386 maintains stable ABI by passing "unsupported" SSE modes in hard
SSE registers, even if original mode does not fit there. This works
quite well for 32bit targets (-msse2 vs -msse), but 64bit targets
assume SSE2 by default, so various paths aren't well tested for
-mno-sse2.

Attached patch fixes one particular problem with -mno-sse2 on 64bit
targets. If the "natural" mode doesn't agree with the mode, choosen by
compiler type system, then compiler decomposes "natural" mode in some
supported mode, breaking various assumptions on how arguments in
"natural" mode are passed around. The patch adopts the same approach
as 32bit targets - it wraps argument in a parallel RTX in a couple of
missed places, tricking the compiler to pass the argument in "natural"
mode. (Please see the comment before gen_reg_or_parallel for further
details).

2012-03-02  Uros Bizjak  <ubizjak@gmail.com>

	PR target/46716
	* config/i386/i386.c (construct_container): Use gen_reg_or_parallel
	to pass the argument in the register of "natural" mode.

testsuite/ChangeLog:

2012-03-02  Uros Bizjak  <ubizjak@gmail.com>

	PR target/46716
	* gcc.target/i386/pr46176.c: New test.

Patch was tested on x86_64-pc-linux-gnu {,-m32}.  Patch was committed
to mainline, is it OK to commit to 4.7 after it opens?

I will backport the patch to other release branches in a couple of days.

Uros.

Comments

Jakub Jelinek March 2, 2012, 4:36 p.m. UTC | #1 
On Fri, Mar 02, 2012 at 05:32:18PM +0100, Uros Bizjak wrote:
> Patch was tested on x86_64-pc-linux-gnu {,-m32}.  Patch was committed
> to mainline, is it OK to commit to 4.7 after it opens?

Have you done any compat testing against compiler without that change
(using ALT_CC_UNDER_TEST=/whatever/gcc ALT_CXX_UNDER_TEST=/whatever/g++ )?
Just to double check it doesn't introduce ABI compatibility issues?

	Jakub
Uros Bizjak March 2, 2012, 4:40 p.m. UTC | #2 
On Fri, Mar 2, 2012 at 5:36 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Mar 02, 2012 at 05:32:18PM +0100, Uros Bizjak wrote:
>> Patch was tested on x86_64-pc-linux-gnu {,-m32}.  Patch was committed
>> to mainline, is it OK to commit to 4.7 after it opens?
>
> Have you done any compat testing against compiler without that change
> (using ALT_CC_UNDER_TEST=/whatever/gcc ALT_CXX_UNDER_TEST=/whatever/g++ )?
> Just to double check it doesn't introduce ABI compatibility issues?

No, but will do so before commit.

Thanks,
Uros.
Uros Bizjak March 2, 2012, 4:53 p.m. UTC | #3 
On Fri, Mar 2, 2012 at 5:40 PM, Uros Bizjak <ubizjak@gmail.com> wrote:

>>> Patch was tested on x86_64-pc-linux-gnu {,-m32}.  Patch was committed
>>> to mainline, is it OK to commit to 4.7 after it opens?
>>
>> Have you done any compat testing against compiler without that change
>> (using ALT_CC_UNDER_TEST=/whatever/gcc ALT_CXX_UNDER_TEST=/whatever/g++ )?
>> Just to double check it doesn't introduce ABI compatibility issues?
>
> No, but will do so before commit.

No surprises on Fedora 16 with:

ALT_CC_UNDER_TEST="/usr/bin/gcc"
ALT_CXX_UNDER_TEST="/usr/bin/g++"

Uros.
diff mbox

Patch

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 184786)
+++ ChangeLog	(working copy)
@@ -231,8 +231,7 @@ 
 	(neon_vcgeu): New insn.
 	(neon_vcgtu): Likewise.
 	* config/arm/neon.ml (s_8_32, u_8_32): New lists.
-	(ops): Unsigned comparison intrinsics call a different
-	builtin.
+	(ops): Unsigned comparison intrinsics call a different builtin.
 
 2012-02-28  Richard Guenther  <rguenther@suse.de>
 
@@ -261,7 +260,7 @@ 
 
 	* config/avr/avr-devices.c (avr_mcu_type): Adjust NULL part
 	of initializer to changes from r184614.
-	
+
 2012-02-28  Richard Guenther  <rguenther@suse.de>
 
 	PR tree-optimization/52395
@@ -330,8 +329,7 @@ 
 2012-02-27  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR target/52352
-	* config/i386/i386.md (*movabs<mode>_1): Enable only for
-	TARGET_LP64.
+	* config/i386/i386.md (*movabs<mode>_1): Enable only for TARGET_LP64.
 	(*movabs<mode>_2): Likewise.
 
 2012-02-27  Jakub Jelinek  <jakub@redhat.com>
Index: testsuite/gcc.target/i386/pr46716.c
===================================================================
--- testsuite/gcc.target/i386/pr46716.c	(revision 0)
+++ testsuite/gcc.target/i386/pr46716.c	(revision 0)
@@ -0,0 +1,33 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -msse -mno-sse2" } */
+/* { dg-require-effective-target sse } */
+
+#include "sse-check.h"
+
+typedef double V __attribute__ ((__vector_size__ (16), __may_alias__));
+typedef union
+{
+  V x;
+  double a[2];
+} u;
+
+#define EMM_FLT8(a) ((double *)&(a))
+
+void __attribute__ ((noinline))
+test (V s1, V s2)
+{
+  if (EMM_FLT8(s1)[0] != EMM_FLT8(s2)[0]
+      || EMM_FLT8(s1)[1] != EMM_FLT8(s2)[1])
+    abort ();
+}
+
+static void
+sse_test (void)
+{
+  u s1;
+
+  s1.a[0] = 1.0;
+  s1.a[1] = 2.0;
+
+  test (s1.x, s1.x);
+}
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog	(revision 184786)
+++ testsuite/ChangeLog	(working copy)
@@ -115,18 +115,15 @@ 
 
 2012-02-28  Kai Tietz  <ktietz@redhat.com>
 
-	* gcc.target/i386/pr46939.c (long): Fix LP64 vs LLP64
-	issue.
+	* gcc.target/i386/pr46939.c (long): Fix LP64 vs LLP64 issue.
 	* gcc.target/i386/pr45352-2.c: Likewise.
-	* gcc.target/i386/bitfield3.c: Add -mno-ms-bitfields for
-	mingw targets.
-	* gcc.target/i386/xop-vshift-1.c(random): Use on mingw
+	* gcc.target/i386/bitfield3.c: Add -mno-ms-bitfields for mingw targets.
+	* gcc.target/i386/xop-vshift-1.c (random): Use on mingw
 	targets instead rand.
 	* gcc.target/i386/sse4_1-blendps-2.c: Likewise.
 	* gcc.target/i386/sse2-mul-1.c: Likewise.
 	* gcc.target/i386/sse4_1-blendps.c: Likewise.
-	* gcc.target/i386/pad-6b.c: Adjust test for x64 mingw
-	target.
+	* gcc.target/i386/pad-6b.c: Adjust test for x64 mingw target.
 	* gcc.target/i386/pad-1.c: Likewise.
 	* gcc.target/i386/pad-9.c: Likewise.
 	* gcc.target/i386/pad-2.c: Likewise.
@@ -212,8 +209,7 @@ 
 
 2012-02-23  Kai Tietz  <ktietz@redhat.com>
 
-	* gcc.dg/pack-test-5.c: Add -mno-ms-bitfields option
-	for mingw-targets.
+	* gcc.dg/pack-test-5.c: Add -mno-ms-bitfields option for mingw-targets.
 	* gcc.dg/Wpadded.c: Likewise.
 	* gcc.dg/bf-ms-layout-2.c: Adjust offsets to fit ms-bitfield
 	structure-layout.
@@ -223,8 +219,7 @@ 
 	targets.
 	* gcc.dg/stack-usage-1.c (SIZE): Provide proper SIZE for x64 mingw
 	target.
-	* gcc.dg/tls/thr-cse-1.c: Provide proper pattern for x64 mingw
-	target.
+	* gcc.dg/tls/thr-cse-1.c: Provide proper pattern for x64 mingw target.
 	* gcc.dg/tls/opt-11.c (memset): Use __extension__ to avoid fail
 	on x64 mingw target.
 	* gcc.dg/bf-ms-attrib.c: Adjust expected size for ms_struct layout.
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 184786)
+++ config/i386/i386.c	(working copy)
@@ -6338,15 +6338,16 @@  construct_container (enum machine_mode mode, enum
       }
   if (n == 2 && regclass[0] == X86_64_SSE_CLASS
       && regclass[1] == X86_64_SSEUP_CLASS && mode != BLKmode)
-    return gen_rtx_REG (mode, SSE_REGNO (sse_regno));
+    return gen_reg_or_parallel (mode, orig_mode,
+				SSE_REGNO (sse_regno));
   if (n == 4
       && regclass[0] == X86_64_SSE_CLASS
       && regclass[1] == X86_64_SSEUP_CLASS
       && regclass[2] == X86_64_SSEUP_CLASS
       && regclass[3] == X86_64_SSEUP_CLASS
       && mode != BLKmode)
-    return gen_rtx_REG (mode, SSE_REGNO (sse_regno));
-
+    return gen_reg_or_parallel (mode, orig_mode,
+				SSE_REGNO (sse_regno));
   if (n == 2
       && regclass[0] == X86_64_X87_CLASS && regclass[1] == X86_64_X87UP_CLASS)
     return gen_rtx_REG (XFmode, FIRST_STACK_REG);
Index: fortran/primary.c
===================================================================
--- fortran/primary.c	(revision 184786)
+++ fortran/primary.c	(working copy)
@@ -1919,7 +1919,7 @@  gfc_match_varspec (gfc_expr *primary, int equiv_fl
 	   && gfc_match_char ('%') == MATCH_YES)
     {
       gfc_error ("Unexpected '%%' for nonderived-type variable '%s' at %C",
-		 sym->name)
+		 sym->name);
       return MATCH_ERROR;
     }