[Bug middle-end/23290] New: Layout changed for structure with single complex field

[Bug middle-end/23290] New: Layout changed for structure with single complex field

[amylaar at gcc dot gnu dot org]

The patch for PR17112:

2004-09-26  Roger Sayle  <roger@eyesopen.com>
            Giovanni Bajo  <giovannibajo@gcc.gnu.org>

        PR middle-end/17112
        * stor-layout.c (compute_record_mode): For records with a single
        field, only use the field's mode if its size matches what we'd
        have choosen for the record ourselves.  This forces the use of
        BLKmode for packed records that don't completely fill a mode.

has changed the way structures are laid out, and as a consequence, the ABI.
I have become aware of this issue when I investigated an sh-elf -m4 regression:
FAIL: gcc.dg/compat/struct-by-value-15 c_compat_y_tst.o compile
sh_gimplify_va_arg_expr replaces a record type with its only element because
that used to be the way we passed these structs; with the unscheduled ABI change,
we end up copying a CDImode variable into a BLKmode one, and ICE.  The ICE
could be avoided in sh_gimplify_va_arg_expr by checking the mode of the type,
and I could also use the original type when building the statements, but the
real issue here is the ABI change.

The structure in question is:

typedef struct
{
  _Complex long long a;
} Scll1;

The mode for field a is computed as

mode_for_size (128, MODE_COMPLEX_INT, 0),

giving CDImode.  This is also the mode that we used to use for SCll1,
thus allowing the struct to be passed as a function argument in registers
(for the SH1..4, this can happen if it was the first or only argument
 elegible to be passed in integer registers).

With the change to compute_record_mode that was made on accounbt of PR17112,
we first compute an integer mode for the size, and only use the member
mode if it matches the integer mode in size.  This integer mode is calculated
layout_type with:

mode_for_size (128, MODE_INT, 1)

I.e. limit is set to 1, meaning that no mode larger than MAX_FIXED_MODE_SIZE
will be returned.  For SH1..SH4, MAX_FIXED_MODE_SIZE is 64, thus mode_for_size
returns BLKmode, the comparison fails, and Scll1 ends up as BLKmode.
The BLKmode layout forces Scll1 values on the stack.
I think for MODE_COMPLEX_INT and MODE_COMPLEX_FLOAT, we should only check
that size of the record matches the mode.

>From looking at the code, I also see that vectors are also laied out using
a limit of 0, so a similar problem arises there.

-- 
           Summary: Layout changed for structure with single complex field
           Product: gcc
           Version: 4.1.0
            Status: UNCONFIRMED
          Keywords: ABI
          Severity: normal
          Priority: P2
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: amylaar at gcc dot gnu dot org
                CC: gcc-bugs at gcc dot gnu dot org,giovannibajo at gcc dot
                    gnu dot org,roger at eyesopen dot com


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

[Bug middle-end/23290] Layout changed for structure with single complex field

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2005-08-09 23:12 -------
So, using limit 0 for when calculating the integer mode for the size would fix 
the regression on sh? 

-- 


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

[Bug middle-end/23290] Layout changed for structure with single complex field

[amylaar at gcc dot gnu dot org]

------- Additional Comments From amylaar at gcc dot gnu dot org  2005-08-15 14:14 -------
(In reply to comment #1)
> So, using limit 0 for when calculating the integer mode for the size would fix 
> the regression on sh? 

Yes, it would fix the problem with CDImode, however, the mode_for_size_tree
call in compute_record_mode is not only used to determine if any mode found
from a single member would be suitable, but also the what mode to use otherwise.
You don't really want to change what you use for the latter.
For vector modes, there is also the added complication that there might be no
integer mode at all wide enough to match the size of the vector mode. 



-- 


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

[Bug middle-end/23290] Layout changed for structure with single complex field

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2005-09-12 13:50 -------
Subject: Bug 23290

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	amylaar@gcc.gnu.org	2005-09-12 13:50:03

Modified files:
	gcc            : ChangeLog stor-layout.c 

Log message:
	PR middle-end/23290
	* stor-layout.c (compute_record_mode): For records with a single
	field, actually check the field's mode size against the type size.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.9937&r2=2.9938
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/stor-layout.c.diff?cvsroot=gcc&r1=1.241&r2=1.242



-- 


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

[Bug middle-end/23290] [4.0 Regression] Layout changed for structure with single complex field

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-09-12 14:52 -------
Confirmed, only a 4.0 regression now.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2005-09-12 14:52:40
               date|                            |
            Summary|Layout changed for structure|[4.0 Regression] Layout
                   |with single complex field   |changed for structure with
                   |                            |single complex field
   Target Milestone|---                         |4.0.2


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

[Bug middle-end/23290] [4.0 Regression] Layout changed for structure with single complex field

[mmitchel at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.0.2                       |4.0.3


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