This is the mail archive of the
mailing list for the GCC project.
[PATCH] Fix bug and pessimization for packed records in Ada
- From: Eric Botcazou <ebotcazou at act-europe dot fr>
- To: gcc-patches at gcc dot gnu dot org
- Date: Thu, 18 Nov 2004 09:58:12 +0200
- Subject: [PATCH] Fix bug and pessimization for packed records in Ada
2003-04-03 Jason Merrill <firstname.lastname@example.org>
* stor-layout.c (do_type_align): New fn, split out from...
(layout_decl): ...here. Do all alignment calculations for
(update_alignment_for_field): Not here.
(start_record_layout, debug_rli): Remove unpadded_align.
* tree.h (struct record_layout_info_s): Remove unpadded_align.
* c-decl.c (finish_enum): Don't set DECL_SIZE, DECL_ALIGN
or DECL_MODE on the CONST_DECLs.
(finish_struct): Don't mess with DECL_ALIGN.
posted at http://gcc.gnu.org/ml/gcc-patches/2003-04/msg00209.html introduced a
bug and a pessimization for the alignment of certain fields inside packed
records in Ada, on STRICT_ALIGNMENT targets.
The typical testcase is:
with Ada.Text_IO; use Ada.Text_IO;
procedure P is
type Time_T is record
Hour : Integer;
type Date_And_Time_T is record
Date : Integer;
Time : Time_T;
Assign_Hour_Of (T : out Time_T)
T.Hour := 44;
Clobber_Hour_Of (DT: out Date_And_Time_T)
DT : Date_And_Time_T;
DT.Time.Hour := 22;
Before the patch, the Time field of the Date_And_Time_T record type was
promoted to 32-bit alignment (in Ada, we try to promote fields in packed
records to their natural alignment as much as possible) so the whole record
was 32-bit aligned. As a consequence, Clobber_Hour_Of was essentially
reduced to a mere addition on the address.
After the patch, the field is not promoted anymore to 32-bit alignment,
although it is given SImode. The consequence is twofold: a temporary is
created in Date_And_Time_T and the middle-end gets confused creating it
because of the SImode of the field, leading to wrong code e.g. on SPARC.
What happens is that the alignment promotion in layout_decl:
/* See if we can use an ordinary integer mode for a bit-field.
Conditions are: a fixed size that is correct for another mode
and occupying a complete byte or bytes on proper boundary. */
if (code == FIELD_DECL && DECL_BIT_FIELD (decl)
&& TYPE_SIZE (type) != 0
&& TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
&& GET_MODE_CLASS (TYPE_MODE (type)) == MODE_INT)
enum machine_mode xmode
= mode_for_size_tree (DECL_SIZE (decl), MODE_INT, 1);
if (xmode != BLKmode && known_align >= GET_MODE_ALIGNMENT (xmode))
DECL_ALIGN (decl) = MAX (GET_MODE_ALIGNMENT (xmode),
DECL_MODE (decl) = xmode;
DECL_BIT_FIELD (decl) = 0;
may be overriden by:
/* If the field is of variable size, we can't misalign it since we
have no way to make a temporary to align the result. But this
isn't an issue if the decl is not addressable. Likewise if it
is of unknown size.
Note that do_type_align may set DECL_USER_ALIGN, so we need to
check old_user_align instead. */
if (DECL_PACKED (decl)
&& (DECL_NONADDRESSABLE_P (decl)
|| DECL_SIZE_UNIT (decl) == 0
|| TREE_CODE (DECL_SIZE_UNIT (decl)) == INTEGER_CST))
DECL_ALIGN (decl) = MIN (DECL_ALIGN (decl), BITS_PER_UNIT);
They were originally (before the patch) in the reverse order.
The patch I'm proposing is also twofold:
- it swaps back the two blocks of code so that we cannot end up in a
"semi-promoted" state anymore; and, in order not to change the default
behaviour, it essentially adds an 'else' between them (that is, we don't
promote if the alignment was reduced to the minimal one because of the
- it adds a language hook to let the front-end override the 'else' if it deems
The patch also gets rid of the set_lang_adjust_rli hook that was added for Ada
3 years ago and silenced (but not removed) by Jason in the same patch, with
the comment (ada/misc.c:gnat_adjust_rli)
/* ??? This code seems to have no actual effect; record_align should already
reflect the largest alignment desired by a field. jason 2003-04-01 */
that I think is correct.
Bootstrapped/regtested on i586-mandrake-linux-gnu. Both problems are
regressions in Ada wrt before Jason's patch so I'm requesting approval for
both mainline and 3.4.x branch.
2004-11-18 ?Eric Botcazou ?<email@example.com>
* langhooks.h (can_promote_packed_bit_field_p): New.
* langhooks.c (lhd_can_promote_packed_bit_field_p): New function.
* langhooks-def.h (lhd_can_promote_packed_bit_field_p): Declare it.
(LANG_HOOKS_CAN_PROMOTE_PACKED_BIT_FIELD_P): Define to it.
(LANG_HOOKS_INITIALIZER): Add LANG_HOOKS_CAN_PROMOTE_PACKED_BIT_FIELD_P.
* stor-layout.c (layout_decl): Attempt to promote a bit-field only
after the default alignment has been computed. ?Do not promote it
if the alignment was forced to the minimal one because of packedness,
unless the language specifically allows it.
(layout_type): Remove invocation of set_lang_adjust_rli.
* tree.h (set_lang_adjust_rli): Remove.
* ada/misc.c (gnat_adjust_rli): Remove.
(gnat_init): Remove invocation of set_lang_adjust_rli.
(gnat_can_promote_packed_bit_field_p): New function.
(LANG_HOOKS_CAN_PROMOTE_PACKED_BIT_FIELD_P): Set it