This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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)))
     {


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]