This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
PR 55438: Incorrect use of BIGGEST_ALIGNMENT
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Sat, 24 Nov 2012 18:33:54 +0000
- Subject: PR 55438: Incorrect use of BIGGEST_ALIGNMENT
In http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00257.html I said:
get_best_mode has various checks to decide what counts as an acceptable
bitfield mode. It actually has two copies of them, with slightly different
alignment checks:
MIN (unit, BIGGEST_ALIGNMENT) > align
vs.
unit <= MIN (align, BIGGEST_ALIGNMENT)
The second looks more correct, since we can't necessarily guarantee
larger alignments than BIGGEST_ALIGNMENT in all cases.
PR 55438 shows why I was wrong. BIGGEST_ALIGNMENT is not (as I thought)
the biggest supported alignment, but the:
Biggest alignment that any data type can require on this machine, in
bits. Note that this is not the biggest alignment that is supported,
just the biggest alignment that, when violated, may cause a fault.
So it's perfectly possible for BIGGEST_ALIGNMENT to be 8 on 32-bit machines.
As things stand, my change causes get_best_mode to reject anything larger
than a byte for that combination, which causes infinite recursion between
store_fixed_bit_field and store_split_bit_field.
There are two problems. First:
if (!bitregion_end_)
{
/* We can assume that any aligned chunk of ALIGN_ bits that overlaps
the bitfield is mapped and won't trap. */
bitregion_end_ = bitpos + bitsize + align_ - 1;
bitregion_end_ -= bitregion_end_ % align_ + 1;
}
is too conservative if the aligment is capped to BIGGEST_ALIGNMENT.
We should be able to guarantee that word-aligned memory is mapped
in at least word-sized chunks. (If you think now is the time to
add a MIN_PAGE_SIZE macro, perhaps with MAX (BITS_PER_WORD,
BIGGEST_ALIGNMENT) as its default, I can do that instead.)
Also, in cases like these, the supposedly conservative:
&& GET_MODE_BITSIZE (mode) <= align
doesn't preserve the cap in the original:
MIN (unit, BIGGEST_ALIGNMENT) > align
Fixed by using GET_MODE_ALIGNMENT instead.
Tested on x86_64-linux-gnu, powerpc64-linux-gnu and cris-elf.
I checked that the output for a set of gcc .ii files didn't
change for the first two. OK to install?
Richard
gcc/
PR middle-end/55438
* stor-layout.c (bit_field_mode_iterator::bit_field_mode_iterator):
Don't limit ALIGN_. Assume that memory is mapped in chunks of at
least word size, regardless of BIGGEST_ALIGNMENT.
(get_best_mode): Use GET_MODE_ALIGNMENT.
Index: gcc/stor-layout.c
===================================================================
--- gcc/stor-layout.c 2012-11-24 10:24:46.000000000 +0000
+++ gcc/stor-layout.c 2012-11-24 10:33:19.011617853 +0000
@@ -2643,15 +2643,17 @@ fixup_unsigned_type (tree type)
unsigned int align, bool volatilep)
: mode_ (GET_CLASS_NARROWEST_MODE (MODE_INT)), bitsize_ (bitsize),
bitpos_ (bitpos), bitregion_start_ (bitregion_start),
- bitregion_end_ (bitregion_end), align_ (MIN (align, BIGGEST_ALIGNMENT)),
+ bitregion_end_ (bitregion_end), align_ (align),
volatilep_ (volatilep), count_ (0)
{
if (!bitregion_end_)
{
- /* We can assume that any aligned chunk of ALIGN_ bits that overlaps
+ /* We can assume that any aligned chunk of UNITS bits that overlaps
the bitfield is mapped and won't trap. */
- bitregion_end_ = bitpos + bitsize + align_ - 1;
- bitregion_end_ -= bitregion_end_ % align_ + 1;
+ unsigned HOST_WIDE_INT units = MIN (align, MAX (BIGGEST_ALIGNMENT,
+ BITS_PER_WORD));
+ bitregion_end_ = bitpos + bitsize + units - 1;
+ bitregion_end_ -= bitregion_end_ % units + 1;
}
}
@@ -2808,7 +2810,7 @@ get_best_mode (int bitsize, int bitpos,
causes store_bit_field to keep a 128-bit memory reference,
so that the final bitfield reference still has a MEM_EXPR
and MEM_OFFSET. */
- && GET_MODE_BITSIZE (mode) <= align
+ && GET_MODE_ALIGNMENT (mode) <= align
&& (largest_mode == VOIDmode
|| GET_MODE_SIZE (mode) <= GET_MODE_SIZE (largest_mode)))
{