> 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
