Re: [5/8] Tweak bitfield alignment handling

[Eric Botcazou <ebotcazou at adacore dot com>]

> This patch replaces:
> 
>       /* Stop if the mode is wider than the alignment of the containing
> 	 object.
> 
> 	 It is tempting to omit the following line unless STRICT_ALIGNMENT
> 	 is true.  But that is incorrect, since if the bitfield uses part
> 	 of 3 bytes and we use a 4-byte mode, we could get a spurious segv
> 	 if the extra 4th byte is past the end of memory.
> 	 (Though at least one Unix compiler ignores this problem:
> 	 that on the Sequent 386 machine.  */
>       if (unit > align_)
> 	break;
> 
> with two checks: one for whether the final byte of the mode is known
> to be mapped, and one for whether the bitfield is sufficiently aligned.
> Later patches in the series rely on this in order not to pessimise
> memory handling.
> 
> However, as described in the patch, I found that extending this
> behaviour to get_best_mode affected code generation for x86_64-linux-gnu
> and powerpc64-linux-gnu, and led to a failure in bb-slp-5.c on both.
> I therefore chickened out and added the original check back to
> get_best_mode.
> 
> I'm certainly interested in any feedback on the comment, but at the
> same time I'd like this series to be a no-op on targets that keep
> the traditional .md patterns.  Any change to get_best_mode is probably
> best done as a separate patch.

I agree with your conservative approach here.

> gcc/
> 	* stor-layout.c (bit_field_mode_iterator::bit_field_mode_iterator):
> 	Set up a default value of bitregion_end_.
> 	(bit_field_mode_iterator::next_mode): Always apply bitregion_end_
> 	check.  Include SLOW_UNALIGNED_ACCESS in the alignment check.
> 	(get_best_mode): Ignore modes that are wider than the alignment.

Fine with me, modulo:

> @@ -2754,6 +2753,62 @@ get_best_mode (int bitsize, int bitpos,
>    enum machine_mode widest_mode = VOIDmode;
>    enum machine_mode mode;
>    while (iter.next_mode (&mode)
> +	 /* ??? For compatiblity, reject modes that are wider than the
> +	    alignment.  This has both advantages and disadvantages.

"For compatibility" is a bit vague (compatibility with what?).  I'd write "For 
historical reasons" or something along these lines.

-- 
Eric Botcazou