[Bug target/43546] New: ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[Bug target/43546] New: ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[zsojka at seznam dot cz]

Command line:
gcc -O1 -m32 -mpreferred-stack-boundary=2 -msseregparm -msse testcase.c

------ testcase.c ------
extern void bar(double);
void foo() { bar(1.0); }
------------------------

Tested revisions:
r157723 - crash
alpha20100318 - segfault (without checking)
r153685 - crash
4.4 r153668 - crash
4.4.3, 4.3.4, 4.2.4, 4.1.2 (gentoo) - OK (without checking)
3.4.6, 3.3.6 - doesn't know -msseregparm

Compiler output:
/mnt/svn/gcc-trunk/binary-157723-lto/bin/gcc -O1 -m32
-mpreferred-stack-boundary=2 -msseregparm -msse testcase.c
testcase.c: In function 'foo':
testcase.c:2:1: internal compiler error: in assign_stack_local_1, at
function.c:353
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.


-- 
           Summary: ICE: in assign_stack_local_1, at function.c:353 with -
                    mpreferred-stack-boundary=2 -msseregparm
           Product: gcc
           Version: 4.4.4
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: zsojka at seznam dot cz
  GCC host triplet: x86_64-pc-linux-gnu
GCC target triplet: i686-pc-linux-gnu


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

[Bug target/43546] [4.4/4.5 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2010-03-26 19:37 -------
Confirmed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |ice-on-valid-code
      Known to fail|                            |4.4.0 4.4.3 4.5.0
      Known to work|                            |4.3.4
   Last reconfirmed|0000-00-00 00:00:00         |2010-03-26 19:37:06
               date|                            |
            Summary|ICE: in                     |[4.4/4.5 Regression] ICE: in
                   |assign_stack_local_1, at    |assign_stack_local_1, at
                   |function.c:353 with -       |function.c:353 with -
                   |mpreferred-stack-boundary=2 |mpreferred-stack-boundary=2
                   |-msseregparm                |-msseregparm
   Target Milestone|---                         |4.4.4


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

[Bug target/43546] [4.4/4.5 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[hjl dot tools at gmail dot com]

------- Comment #2 from hjl dot tools at gmail dot com  2010-03-26 23:26 -------
We are trying to store DFmode with SFmode alignment since
compress_float_constant converts 1.0DF to 1.0SF. This patch
works around the problem:

--
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 35b4c49..14879c2 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -25828,7 +25828,8 @@ ix86_rtx_costs (rtx x, int code, int outer_code_i, int
*total, bool speed)
       return false;

     case FLOAT_EXTEND:
-      if (!(SSE_FLOAT_MODE_P (mode) && TARGET_SSE_MATH))
+      if (!(SSE_FLOAT_MODE_P (mode)
+      && (TARGET_SSE_MATH || TARGET_SSEREGPARM)))
   *total = 0;
       return false;
--

-msseregparm is very similar to -mfpmath=sse for function parameters.
Personally, I think we should allow -msseregparm only if -mfpmath=sse
is used.

---
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 35b4c49..9f6d947 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -3306,10 +3306,11 @@ override_options (bool main_args_p)
        }
     }

-  /* Accept -msseregparm only if at least SSE support is enabled.  */
+  /* Accept -msseregparm only if at least SSE fpmath support is enabled.  */
   if (TARGET_SSEREGPARM
-      && ! TARGET_SSE)
-    error ("%ssseregparm%s used without SSE enabled", prefix, suffix);
+      && ! TARGET_SSE_MATH)
+    error ("%ssseregparm%s used without SSE fpmath enabled",
+          prefix, suffix);

   ix86_fpmath = TARGET_FPMATH_DEFAULT;
   if (ix86_fpmath_string != 0)
@@ -4620,16 +4621,16 @@ ix86_function_sseregparm (const_tree type, const_tree
decl, bool warn)
   if (TARGET_SSEREGPARM
       || (type && lookup_attribute ("sseregparm", TYPE_ATTRIBUTES (type))))
     {
-      if (!TARGET_SSE)
+      if (!TARGET_SSEREGPARM)
        {
          if (warn)
            {
              if (decl)
                error ("Calling %qD with attribute sseregparm without "
-                      "SSE/SSE2 enabled", decl);
+                      "SSE fpmath enabled", decl);
              else
                error ("Calling %qT with attribute sseregparm without "
-                      "SSE/SSE2 enabled", type);
+                      "SSE fpmath enabled", type);
            }
          return 0;
        }
--


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl dot tools at gmail dot
                   |                            |com


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

[Bug target/43546] [4.4/4.5 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[hjl dot tools at gmail dot com]

------- Comment #3 from hjl dot tools at gmail dot com  2010-03-26 23:29 -------
Created an attachment (id=20219)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20219&action=view)
A patch to check TARGET_SSE_MATH instead of TARGET_SSE


-- 


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

[Bug target/43546] [4.4/4.5 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2010-03-27 12:53 -------
No, sseregparm should be perfectly fine with x87 math - it only needs
(obviously) SSE registers available.  It's an ABI switch like regparm,
whether math is done using SSE registers or x87 math doesn't and shouldn't
matter.

The workaround looks odd - it can't be the real solution.


-- 


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

[Bug target/43546] [4.4/4.5 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[hjl dot tools at gmail dot com]

------- Comment #5 from hjl dot tools at gmail dot com  2010-03-27 14:11 -------
(In reply to comment #4)
> No, sseregparm should be perfectly fine with x87 math - it only needs
> (obviously) SSE registers available.  It's an ABI switch like regparm,
> whether math is done using SSE registers or x87 math doesn't and shouldn't
> matter.
> 

Then you need to exam all code paths with -msseregparm to see
if TARGET_SSEREGPARM check is needed when TARGET_SSE_MATH is
checked. 


-- 


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

[Bug target/43546] [4.4/4.5 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2010-03-27 15:23 -------
(In reply to comment #5)
> (In reply to comment #4)
> > No, sseregparm should be perfectly fine with x87 math - it only needs
> > (obviously) SSE registers available.  It's an ABI switch like regparm,
> > whether math is done using SSE registers or x87 math doesn't and shouldn't
> > matter.
> > 
> 
> Then you need to exam all code paths with -msseregparm to see
> if TARGET_SSEREGPARM check is needed when TARGET_SSE_MATH is
> checked. 

I don't follow.  TARGET_SSEREGPARM does not mean you'll do SSE math.
It merely says that you can expect incoming arguments in SSE registers
instead of on stack.  For -mpreferred-stack-boundary=2 how is the stack
for incoming arguments of double type aligned?  Also if we'd ever spill
a double to stack with x87 math we'd run into exactly the same assert?


-- 


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

[Bug target/43546] [4.4/4.5 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[hjl dot tools at gmail dot com]

------- Comment #7 from hjl dot tools at gmail dot com  2010-03-27 15:37 -------
(In reply to comment #6)
> I don't follow.  TARGET_SSEREGPARM does not mean you'll do SSE math.
> It merely says that you can expect incoming arguments in SSE registers
> instead of on stack.  For -mpreferred-stack-boundary=2 how is the stack
> for incoming arguments of double type aligned?  Also if we'd ever spill
> a double to stack with x87 math we'd run into exactly the same assert?
> 

TARGET_SSEREGPARM may put DF/SF in xmm registers for
function parameters, which is very similar to TARGET_SSE_MATH
which uses xmm for DF/SF, but more than just function parameters.

BTW, "-mpreferred-stack-boundary=2 -msse" never worked before
gcc 4.4. We have a bunch of related bug reports:

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


-- 


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

[Bug target/43546] [4.4/4.5 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[rguenth at gcc dot gnu dot org]

------- Comment #8 from rguenth at gcc dot gnu dot org  2010-03-27 16:38 -------
Just to clarify, the issue is that

(insn 6 5 7 2 t.i:2 (set (reg:DF 21 xmm0)
        (float_extend:DF (mem/u/c/i:SF (symbol_ref/u:SI ("*.LC0") [flags 0x2])
[0 S4 A32]))) 99 {*extendsfdf2_i387} (expr_list:REG_EQUAL (const_double:DF
-2147483648 [0x80000000] 1.0e+0 [0x0.8p+1])
        (nil)))

with

(define_insn "*extendsfdf2_i387"
  [(set (match_operand:DF 0 "nonimmediate_operand" "=f,m")
        (float_extend:DF (match_operand:SF 1 "nonimmediate_operand" "fm,f")))]
  "TARGET_80387"
  "* return output_387_reg_move (insn, operands);"
  [(set_attr "type" "fmov")
   (set_attr "mode" "SF,XF")])

requires a reload for the x87 register to xmm0 move.  The

(define_insn "*extendsfdf2_sse"
  [(set (match_operand:DF 0 "nonimmediate_operand" "=x")
        (float_extend:DF (match_operand:SF 1 "nonimmediate_operand" "xm")))]
  "TARGET_SSE2 && TARGET_SSE_MATH"
  "%vcvtss2sd\t{%1, %d0|%d0, %1}"
  [(set_attr "type" "ssecvt")
   (set_attr "prefix" "maybe_vex")
   (set_attr "mode" "DF")])

alternative is not enabled, both because just -msse is supplied and
SSE math is not enabled.  But I fail to see why we can't go through
unaligned memory if the user asks us to - which puts the finger
at the assert that triggers (and maybe reload if it can not properly
deal with less aligned stack slots).

Testcase that doesn't need -msseregparm:

extern void __attribute__((sseregparm)) bar(double);
void foo() { bar(1.0); }

commenting the assert yields odd

foo:
        pushl   %ebp
        movl    %esp, %ebp
        subl    $8, %esp
        fld1
        fstpl   -8(%ebp)
        movlps  -8(%ebp), %xmm0
        call    bar
        leave
        ret

obviously w/o -msse2 there's no movsd, thus the odd code.

And the handed out secondary memory has correct alignment:

insn 6 5 11 2 t.i:2 (set (reg:DF 8 st)
        (float_extend:DF (mem/u/c/i:SF (symbol_ref/u:SI ("*.LC0") [flags 0x2])
[0 S4 A32]))) 99 {*extendsfdf2_i387} (expr_list:REG_EQUAL (const_double:DF
-2147483648 [0x80000000] 1.0e+0 [0x0.8p+1])
        (nil)))

(insn 11 6 12 2 t.i:2 (set (mem/c:DF (plus:SI (reg/f:SI 6 bp)
                (const_int -8 [0xfffffff8])) [0 S8 A32])
        (reg:DF 8 st)) 74 {*movdf_nointeger} (nil))

(insn 12 11 7 2 t.i:2 (set (reg:DF 21 xmm0)
        (mem/c:DF (plus:SI (reg/f:SI 6 bp)
                (const_int -8 [0xfffffff8])) [0 S8 A32])) 74 {*movdf_nointeger}
(nil))

(call_insn 7 12 10 2 t.i:2 (call (mem:QI (symbol_ref:SI ("bar") [flags 0x41] 
<function_decl 0xb77a4c80 bar>) [0 S1 A8])
        (const_int 0 [0x0])) 484 {*call_0} (nil)
    (expr_list:REG_DEP_TRUE (use (reg:DF 21 xmm0))
        (nil)))

so why are we using assign_stack_local here and not assign_stack_local_1 (...,
true)?  Here, at

#3  0x0846d729 in get_secondary_mem (x=0xb77a96f0, mode=DFmode, opnum=0, 
    type=RELOAD_FOR_OUTPUT) at /home/richard/src/trunk/gcc/reload.c:608
608             = assign_stack_local (mode, GET_MODE_SIZE (mode), 0);
(gdb) l
603         {
604     #ifdef SECONDARY_MEMORY_NEEDED_RTX
605           secondary_memlocs[(int) mode] = SECONDARY_MEMORY_NEEDED_RTX
(mode);
606     #else
607           secondary_memlocs[(int) mode]
608             = assign_stack_local (mode, GET_MODE_SIZE (mode), 0);
609     #endif
610         }


-- 

rguenth at gcc dot gnu dot org changed:

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


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

[Bug target/43546] [4.4/4.5 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2010-03-27 16:39 -------
In fact reduce_alignment_ok is _only_ used in the assert.


-- 


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

[Bug target/43546] [4.4/4.5/4.6 Regression] ICE: in assign_stack_local_1, at function.c:353 with -mpreferred-stack-boundary=2 -msseregparm

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.4                       |4.4.5


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