[Bug c/46716] New: bad code generated with -mno-sse2 -m64

[Bug c/46716] New: bad code generated with -mno-sse2 -m64

[mathog at caltech dot edu]

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

           Summary: bad code generated with -mno-sse2 -m64
           Product: gcc
           Version: 4.4.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: mathog@caltech.edu


The following small program generates incorrect output when compiled with
the flags -mno-sse2 -m64 on a 64 bit system.  Output is correct for -m32
or -msse2 -m64.  The problem manifests as the lower double of the second
parameter passed into test() being replaced by the value of the second double
from the first parameter.

% cat /etc/release
Mandriva Linux release 2010.0 (Official) for x86_64
% cat /proc/cpuinfo | head -10
processor       : 0
vendor_id       : AuthenticAMD
cpu family      : 15
model           : 33
model name      : Dual Core AMD Opteron(tm) Processor 280
stepping        : 2
cpu MHz         : 1000.000
cache size      : 1024 KB
physical id     : 0
siblings        : 2
% gcc --version
gcc (GCC) 4.4.1

example:

% gcc -msse -mno-sse2 -I. -m64   -o foo gccprob2.c
% ./foo
s1      1.000000 2.000000
test s1 1.000000 2.000000
test s2 2.000000 2.000000
% gcc -msse -mno-sse2 -I. -m32   -o foo gccprob2.c
OR
% gcc -msse -msse2 -I. -m64   -o foo gccprob2.c
s1      1.000000 2.000000
test s1 1.000000 2.000000
test s2 1.000000 2.000000

*************************************************
#include <stdio.h> /* for printf */
typedef double    __m128d __attribute__ ((__vector_size__ (16),
__may_alias__));
typedef union
{
  __m128d x;
  double a[2];
} union128d;
#define EMM_FLT8(a)    ((double *)&(a))

void test ( __m128d s1, __m128d s2)
{
printf("test s1 %lf %lf\n",EMM_FLT8(s1)[0],EMM_FLT8(s1)[1]);
printf("test s2 %lf %lf\n",EMM_FLT8(s2)[0],EMM_FLT8(s2)[1]);
}

int main (void)
{
__attribute__ ((aligned (16)))  union128d s1;
  s1.a[0] = 1.0;
  s1.a[1] = 2.0;
printf("s1      %lf %lf\n",s1.a[0],s1.a[1]);
  test (s1.x, s1.x);
}
**************************************************

[Bug target/46716] [4.3/4.4/4.5/4.6 Regression] bad code generated with -mno-sse2 -m64

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2010.11.30 08:18:38
                 CC|                            |jakub at gcc dot gnu.org
      Known to work|                            |3.4.6
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |
   Target Milestone|---                         |4.3.6
            Summary|bad code generated with     |[4.3/4.4/4.5/4.6
                   |-mno-sse2 -m64              |Regression] bad code
                   |                            |generated with -mno-sse2
                   |                            |-m64
     Ever Confirmed|0                           |1
      Known to fail|                            |4.1.2, 4.3.5, 4.4.4, 4.6.0

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-11-30 08:18:38 UTC ---
Confirmed, will look at it.

[Bug target/46716] [4.3/4.4/4.5/4.6 Regression] bad code generated with -mno-sse2 -m64

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-11-30 12:22:26 UTC ---
I think there is a dup somewhere.  Note that IMHO disabling sse2 makes the
code suspicious as it cannot fulfill the ABI (I think the kernel uses
-mno-sse2 to disable use of SSE registers, not just non-move SSE instructions
for example).

[Bug target/46716] [4.3/4.4/4.5/4.6 Regression] bad code generated with -mno-sse2 -m64

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
                 CC|                            |hjl at gcc dot gnu.org,
                   |                            |hubicka at gcc dot gnu.org
         AssignedTo|jakub at gcc dot gnu.org    |unassigned at gcc dot
                   |                            |gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-11-30 12:35:56 UTC ---
Well, apparently -mno-sse2 doesn't disable completely SSE2 use, vectors are
still passed in %xmm* registers for -m64.  See i386.c (type_natural_mode).
The problem is that then we have a parameter, where data->nominal_mode (and
passed_mode and promoted_mode) is BLKmode, but data->entry_parm is
(reg:V2DF 21 xmm0 [ s1 ]).  assign_parm_setup_block_p because of the BLKmode
nominal_mode returns true and assign_parm_setup_block copies it in wordsize
(DImode here) parts, incrementing REGNO for each part.
So, we end up passing first half of first parameter in %xmm0, second half in
%xmm1, first half of second parameter in %xmm1 and second half of second
parameter in %xmm2 (both on the caller and callee side).

I guess generic vector support in 3.4 was even more broken and it just didn't
care and used V2DFmode for the type even with -mno-sse2.

And of course using -mno-sse2 in x86-64 on code that uses vectors makes no
sense at all.

Anyway, not sure how to convince the middle-end to handle this "right" for
whatever definition of right, without risking breaking other targets too much.

[Bug target/46716] [4.3/4.4/4.5/4.6 Regression] bad code generated with -mno-sse2 -m64

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|hjl at gcc dot gnu.org      |hjl.tools at gmail dot com,
                   |                            |ubizjak at gmail dot com

--- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> 2010-11-30 13:40:23 UTC ---
From

http://gcc.gnu.org/ml/gcc/2010-11/msg00650.html

I think this is a compiler bug in the i386 backend.  The
classify_argument function uses X86_64_SSEUP_CLASS for V2DFmode, and
examine_argument counts that as requiring a single SSE register.
However, since the SSE2 instructions are not available, the argument is
split into two SSE registers.  The result is that the first argument is
passed in %xmm0/%xmm1, and the second argument is passed in %xmm1/%xmm2.
That is, the arguments overlap, leading to the incorrect result.

Basically, the 64-bit calling convention support assumes that the SSE2
instructions are always available, and silently fails when -mno-sse2 is
used.  I don't really have an opinion as to whether the compiler needs
to support this case correctly, but I think that clearly it must not
silently fail.

[Bug target/46716] [4.3/4.4/4.5/4.6 Regression] bad code generated with -mno-sse2 -m64

[mathog at caltech dot edu]

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

--- Comment #5 from David Mathog <mathog at caltech dot edu> 2010-11-30 17:25:01 UTC ---
A (long) side note on how I found this bug - in partial answer to the obvious
question - why would anybody run with -mno-sse2 on an X86_64 platform?  

We have a cluster of Athlon MP machines and one of the applications that run
there is Sean Eddy's HMMER which is used to search a database call PFAMDIR. 
With version 3 of that software PFAMDIR changed format to only work with the
newer software.  HMMER 3 has a reference version (portable) and an SSE (really
SSE2 version).  I found that the reference version did not give exactly the
same answers as the SSE version, so it wasn't going to be possible to refine
the reference version for that platform and get the exact same results as
everybody else, but the SSE2 version could not run on the target since that
processor has no SSE2.  Since I need to make this work on those old machines I
wrote an SSE2 emulator which is a replacement emmintrin.h (latest version here)

  http://saf.bio.caltech.edu/pub/software/linux_or_unix_tools/soft_emmintrin.h

This would be used instead of the native SSE2 (in theory) by dropping it into a
directory as emmintrin.h and using "-I. -mno-sse2 -DSOFT_SSE2" on the gcc
compile line.  The idea being to get the SSE2 version running on the target and
then optimize from that version for the target, retaining the same numerical
results along the way.  

Since there was a lot of recompiling/retesting during development I used my
fastest machine, which happened to be an Opteron running an X86_64 OS.  To
exercise the software SSE2 code all of the SSE2 tests in the gcc testsuite were
run, and these triggered the present bug due to the default implicit -m64.

I also have done some preliminary work on a soft_xmmintrin.h, but have my
doubts that it is possible to use that successfully in combination with the gcc
vector extension, since many strange things happen when -mno-sse is added to
the command line.  It seems that the gcc vector extension is very much
intertwined with SSE on X86 platforms and perhaps cannot be fully separated
from it.  (A point that is not made at all clear in the documentation.) 

Additionally, with -msse -mno-sse2 -m32 and levels of optimization above -O0
complex expression like this is used in a real program (with multiple _mm
functions used, it does not show up in the testsuite with single _mm function
calls):

#define EMMMIN(a,b)      ((a)<(b)?(a):(b))
#define EMM_UINT1(a)   ((unsigned char *)&(a))
/* vector operation:  returns the minimum of each pair of the
16 8 bit unsigned integers from __A, __B */
static __inline __m128i __attribute__((__always_inline__))
_mm_min_epu8 (__m128i __A, __m128i __B)
{
  __v16qi __tmp={  EMMMIN(EMM_UINT1(__A)[ 0],EMM_UINT1(__B)[ 0]), 
                   EMMMIN(EMM_UINT1(__A)[ 1],EMM_UINT1(__B)[ 1]),
                   EMMMIN(EMM_UINT1(__A)[ 2],EMM_UINT1(__B)[ 2]),
                   EMMMIN(EMM_UINT1(__A)[ 3],EMM_UINT1(__B)[ 3]),
                   EMMMIN(EMM_UINT1(__A)[ 4],EMM_UINT1(__B)[ 4]),
                   EMMMIN(EMM_UINT1(__A)[ 5],EMM_UINT1(__B)[ 5]),
                   EMMMIN(EMM_UINT1(__A)[ 6],EMM_UINT1(__B)[ 6]),
                   EMMMIN(EMM_UINT1(__A)[ 7],EMM_UINT1(__B)[ 7]), 
                   EMMMIN(EMM_UINT1(__A)[ 8],EMM_UINT1(__B)[ 8]),
                   EMMMIN(EMM_UINT1(__A)[ 9],EMM_UINT1(__B)[ 9]),
                   EMMMIN(EMM_UINT1(__A)[10],EMM_UINT1(__B)[10]),
                   EMMMIN(EMM_UINT1(__A)[11],EMM_UINT1(__B)[11]),
                   EMMMIN(EMM_UINT1(__A)[12],EMM_UINT1(__B)[12]),
                   EMMMIN(EMM_UINT1(__A)[13],EMM_UINT1(__B)[13]),
                   EMMMIN(EMM_UINT1(__A)[14],EMM_UINT1(__B)[14]),
                   EMMMIN(EMM_UINT1(__A)[15],EMM_UINT1(__B)[15])};
  return (__m128i)__tmp;
}

often result in this sort of compiler error:

./msvfilter.c:208: error: unable to find a register to spill in class
'GENERAL_REGS'
./msvfilter.c:208: error: this is the insn:
(insn 1944 1943 1945 46 ../../easel/emmintrin.h:2348 (set
(strict_low_part (subreg:HI (reg:TI 1239) 0))
        (mem:HI (reg/f:SI 96 [ pretmp.1031 ]) [13 S2 A16])) 47
{*movstricthi_1} (nil))
./msvfilter.c:208: confused by earlier errors, bailing out

Simpler (fewer vector elements, less logic) functions did not do this, although
it may be that they would have had I been able to get past the first error. 
This is, I suspect, again related to an implicit use of SSE2 registers even
though -mno-sse2 had been specified.  This type of error shows up even when
-m32 is specified, so maybe it has a different origin.  In any case, rewriting
the expressions as follows seems to have eliminated this problem even for -O4,
and the primary change was the replacement of the vector {} notation to set the
(same) values.

typedef union {
 __m128i             vi;
 __m128d             vd;
 __m128              vf;
  double             f8[2];
  float              f4[4];
  long long          i8[2];
  int                i4[4];
  short              i2[8];
  char               i1[16];
  unsigned long long u8[2];
  unsigned int       u4[4];
  unsigned short     u2[8];
  unsigned char      u1[16];
} __uni16;
#define EMM_UINT1(a)   (((__uni16)(a)).u1)
#define EMMMIN(a,b)      ((a)<(b)?(a):(b))


/* vector operation:  returns the minimum of each pair of the
16 8 bit unsigned integers from __A, __B */
static __inline __m128i __attribute__((__always_inline__))
_mm_min_epu8 (__m128i __A, __m128i __B)
{
  __uni16 __tmp;
    __tmp.u1[ 0] =  EMMMIN(EMM_UINT1(__A)[ 0],EMM_UINT1(__B)[ 0]);
    __tmp.u1[ 1] =  EMMMIN(EMM_UINT1(__A)[ 1],EMM_UINT1(__B)[ 1]);
    __tmp.u1[ 2] =  EMMMIN(EMM_UINT1(__A)[ 2],EMM_UINT1(__B)[ 2]);
    __tmp.u1[ 3] =  EMMMIN(EMM_UINT1(__A)[ 3],EMM_UINT1(__B)[ 3]);
    __tmp.u1[ 4] =  EMMMIN(EMM_UINT1(__A)[ 4],EMM_UINT1(__B)[ 4]);
    __tmp.u1[ 5] =  EMMMIN(EMM_UINT1(__A)[ 5],EMM_UINT1(__B)[ 5]);
    __tmp.u1[ 6] =  EMMMIN(EMM_UINT1(__A)[ 6],EMM_UINT1(__B)[ 6]);
    __tmp.u1[ 7] =  EMMMIN(EMM_UINT1(__A)[ 7],EMM_UINT1(__B)[ 7]);
    __tmp.u1[ 8] =  EMMMIN(EMM_UINT1(__A)[ 8],EMM_UINT1(__B)[ 8]);
    __tmp.u1[ 9] =  EMMMIN(EMM_UINT1(__A)[ 9],EMM_UINT1(__B)[ 9]);
    __tmp.u1[10] =  EMMMIN(EMM_UINT1(__A)[10],EMM_UINT1(__B)[10]);
    __tmp.u1[11] =  EMMMIN(EMM_UINT1(__A)[11],EMM_UINT1(__B)[11]);
    __tmp.u1[12] =  EMMMIN(EMM_UINT1(__A)[12],EMM_UINT1(__B)[12]);
    __tmp.u1[13] =  EMMMIN(EMM_UINT1(__A)[13],EMM_UINT1(__B)[13]);
    __tmp.u1[14] =  EMMMIN(EMM_UINT1(__A)[14],EMM_UINT1(__B)[14]);
    __tmp.u1[15] =  EMMMIN(EMM_UINT1(__A)[15],EMM_UINT1(__B)[15]);
  return __tmp.vi;
}

[Bug target/46716] [4.3/4.4/4.5/4.6 Regression] bad code generated with -mno-sse2 -m64

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |accepts-invalid

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-12-05 12:03:09 UTC ---
For sake of the ABI we either need to ignore -mno-sse2 for calling conventions
or non-silently fail, best with sorry ("cannot use SSE argument passing without
SSE registers") or similar.  If we ignore -mno-sse2 for arguments we probably
have to at least allow spilling SSE regs to memory with all the fun
side-effects.

I don't think this is actually a "regression", just a failure to diagnose
-mno-sse[2] problems from the start.

>From ml discussion this is an accepts-invalid bug.

Leaving at P3 for now.

[Bug target/46716] [4.3/4.4/4.5/4.6 Regression] bad code generated with -mno-sse2 -m64

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug target/46716] [4.3/4.4/4.5/4.6/4.7 Regression] wrong code generated with -mno-sse2 -m64

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.3.6                       |4.4.7

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-27 12:13:32 UTC ---
4.3 branch is being closed, moving to 4.4.7 target.

[Bug target/46716] [4.4/4.5/4.6/4.7 Regression] wrong code generated with -mno-sse2 -m64

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.4/4.5/4.6/4.7/4.8        |[4.4/4.5/4.6/4.7
                   |Regression] wrong code      |Regression] wrong code
                   |generated with -mno-sse2    |generated with -mno-sse2
                   |-m64                        |-m64

--- Comment #8 from Uros Bizjak <ubizjak at gmail dot com> 2012-03-02 14:35:24 UTC ---
This is just a matter of always forcing the "natural" mode for the register,
even when type system does not agree with this. See the comment for
gen_reg_or_parallel on this issue.

I am testing following patch:

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c  (revision 184774)
+++ 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);

[Bug target/46716] [4.4/4.5/4.6/4.7/4.8 Regression] wrong code generated with -mno-sse2 -m64

[uros at gcc dot gnu.org]

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

--- Comment #9 from uros at gcc dot gnu.org 2012-03-02 17:03:44 UTC ---
Author: uros
Date: Fri Mar  2 17:03:36 2012
New Revision: 184802

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184802
Log:
    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:

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


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr46716.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/testsuite/ChangeLog

[Bug target/46716] [4.5/4.6/4.7/4.8 Regression] wrong code generated with -mno-sse2 -m64

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.7                       |4.5.4

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-13 12:46:26 UTC ---
4.4 branch is being closed, moving to 4.5.4 target.

[Bug target/46716] [4.5/4.6/4.7/4.8 Regression] wrong code generated with -mno-sse2 -m64

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.4                       |4.6.4

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-02 12:09:18 UTC ---
The 4.5 branch is being closed, adjusting target milestone.

[Bug target/46716] [4.7/4.8/4.9 Regression] wrong code generated with -mno-sse2 -m64

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.6.4                       |4.7.4

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-12 15:16:24 UTC ---
GCC 4.6.4 has been released and the branch has been closed.

[Bug target/46716] [4.7/4.8/4.9 Regression] wrong code generated with -mno-sse2 -m64

[law at redhat dot com]

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

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |law at redhat dot com
         Resolution|---                         |FIXED

--- Comment #13 from Jeffrey A. Law <law at redhat dot com> ---
Fixed on the trunk some time ago.