Re: PR 55438: Incorrect use of BIGGEST_ALIGNMENT

[Eric Botcazou <ebotcazou at adacore dot com>]

> 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.

That means that the Sequent check was flawed, doesn't it?  It also seems that 
the entire business of alignment with size comparisons is dubious.

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

So this will fix the Sequent check with a conservative cap of BITS_PER_WORD.
That's reasonable, I think.

> 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.

Note that the comment just above needs to be adjusted then.  What about the 
similar check in next_mode?

      /* Stop if the mode requires too much alignment.  */
      if (unit > align_ && SLOW_UNALIGNED_ACCESS (mode_, align_))
	break;

It seems to me that we should change it as well.

-- 
Eric Botcazou