This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: C++ structure layout breakage
- To: gcc-patches at gcc dot gnu dot org
- Subject: Re: C++ structure layout breakage
- From: Jim Wilson <wilson at cygnus dot com>
- Date: Tue, 24 Oct 2000 18:30:27 -0700
I of course found yet another problem. This one showed up while trying to
build a ia64-linux GNU libc with my last proposed patch. The simplified
testcase is this:
union foo
{
long double ld;
double d;
};
sub ()
{
union foo bar;
bar.d = 0.0;
}
This gets an abort in store_bit_field because we are trying to insert a
DFmode value into an TFmode (or XFmode) object, and that just doesn't work,
as we don't allow (subreg:DF (reg:TF)). This is likely the original problem
that Kenner tried to fix with his March 25 change to compute_record_mode.
The real problem here is a bug in Mark Mitchell's March 14 change.
2000-03-14 Mark Mitchell <mark@codesourcery.com>
* stor-layout.c (layout_union): Remove.
(layout_union_field): New function, split out from layout_union.
(finish_union_layout): Likewise.
(layout_field): Handle unions by calling layout_union_field.
(finish_record_layout): Handle unions.
(layout_type): Combine RECORD_TYPE, UNION_TYPE, and
QUAL_UNION_TYPE handling.
This commonized the handling of records and unions. Unfortunately, Mark
missed the fact that the code setting TYPE_MODE for records and unions was
different, and this difference is important. With gcc-2.8.1, the union
above gets BLKmode. After Mark's change, it got TFmode which doesn't work.
Since changing the default mode of unions is an ABI change, this is undesirable
and the best fix is to go back to the original BLKmode.
I now have found problems with 5 separate patches, and have good explanations
for all of them. Richard Henderson broke store_bit_field for FP modes. Mark
Mitchell broke TYPE_MODE of unions. Kenner's Mar 25 patch incorrectly tried
to fix Mark's bug. Kenner's Aug 18 patch incorrectly tried to fix Richard's
bug. Kenner's Oct 20 patch fixed a problem with the Mar 25 patch. I have
better fixes for the original two bugs, so none of Kenner's 3 changes are
needed anymore.
This patch has been tested with an ia32-linux make bootstrap and make check.
Also, on ia64-linux, with a GNU libc build and make check, a linux kernel
build, and I've got a toolchain bootstrap running but didn't want to wait for
it as it will take a while.
2000-10-24 Jim Wilson <wilson@cygnus.com>
* expmed.c (store_bit_field): Move integer pun code down after
code that calls emit_move_insn for entire register move.
* stor-layout.c (compute_record_mode): Revert Mar 25, Aug 18, and
Oct 20 changes. Only store mode in TYPE_MODE if RECORD_TYPE.
Index: expmed.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/expmed.c,v
retrieving revision 1.67
diff -p -r1.67 expmed.c
*** expmed.c 2000/10/19 15:48:22 1.67
--- expmed.c 2000/10/24 22:58:03
*************** store_bit_field (str_rtx, bitsize, bitnu
*** 269,289 ****
op0 = SUBREG_REG (op0);
}
- /* Make sure we are playing with integral modes. Pun with subregs
- if we aren't. */
- {
- enum machine_mode imode = int_mode_for_mode (GET_MODE (op0));
- if (imode != GET_MODE (op0))
- {
- if (GET_CODE (op0) == MEM)
- op0 = change_address (op0, imode, NULL_RTX);
- else if (imode != BLKmode)
- op0 = gen_lowpart (imode, op0);
- else
- abort ();
- }
- }
-
/* If OP0 is a register, BITPOS must count within a word.
But as we have it, it counts within whatever size OP0 now has.
On a bigendian machine, these are not the same, so convert. */
--- 269,274 ----
*************** store_bit_field (str_rtx, bitsize, bitnu
*** 336,341 ****
--- 321,343 ----
emit_move_insn (op0, value);
return value;
}
+
+ /* Make sure we are playing with integral modes. Pun with subregs
+ if we aren't. This must come after the entire register case above,
+ since that case is valid for any mode. The following cases are only
+ valid for integral modes. */
+ {
+ enum machine_mode imode = int_mode_for_mode (GET_MODE (op0));
+ if (imode != GET_MODE (op0))
+ {
+ if (GET_CODE (op0) == MEM)
+ op0 = change_address (op0, imode, NULL_RTX);
+ else if (imode != BLKmode)
+ op0 = gen_lowpart (imode, op0);
+ else
+ abort ();
+ }
+ }
/* Storing an lsb-aligned field in a register
can be done with a movestrict instruction. */
Index: stor-layout.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/stor-layout.c,v
retrieving revision 1.86
diff -p -r1.86 stor-layout.c
*** stor-layout.c 2000/10/20 19:42:40 1.86
--- stor-layout.c 2000/10/24 22:58:04
*************** compute_record_mode (type)
*** 1065,1076 ****
/* If this field is the whole struct, remember its mode so
that, say, we can put a double in a class into a DF
! register instead of forcing it to live in the stack. However,
! we don't support using such a mode if there is no integer mode
! of the same size, so don't set it here. */
! if (field == TYPE_FIELDS (type) && TREE_CHAIN (field) == 0
! && int_mode_for_mode (DECL_MODE (field)) != BLKmode
! && operand_equal_p (DECL_SIZE (field), TYPE_SIZE (type), 1))
mode = DECL_MODE (field);
#ifdef STRUCT_FORCE_BLK
--- 1065,1072 ----
/* If this field is the whole struct, remember its mode so
that, say, we can put a double in a class into a DF
! register instead of forcing it to live in the stack. */
! if (simple_cst_equal (TYPE_SIZE (type), DECL_SIZE (field)))
mode = DECL_MODE (field);
#ifdef STRUCT_FORCE_BLK
*************** compute_record_mode (type)
*** 1081,1088 ****
#endif /* STRUCT_FORCE_BLK */
}
! if (mode != VOIDmode)
! /* We only have one real field; use its mode. */
TYPE_MODE (type) = mode;
else
TYPE_MODE (type) = mode_for_size_tree (TYPE_SIZE (type), MODE_INT, 1);
--- 1077,1085 ----
#endif /* STRUCT_FORCE_BLK */
}
! /* If we only have one real field; use its mode. This only applies to
! RECORD_TYPE. This does not apply to unions. */
! if (TREE_CODE (type) == RECORD_TYPE && mode != VOIDmode)
TYPE_MODE (type) = mode;
else
TYPE_MODE (type) = mode_for_size_tree (TYPE_SIZE (type), MODE_INT, 1);