On Fri, 10 Jun 2016, Segher Boessenkool wrote:
> Currently, optimize_bit_field_compare reads the bitfield in word_mode
> if it can. If the bit field is normally accessed in a smaller mode,
> this might be a violation of the memory model, although the "extra"
> part of the read is not used. But also, previous stores to the bit
> field will have been done in the smaller mode, and then bigger loads
> from it cause a LHS problem.
>
> Bootstrapped and regchecked on powerpc64-linux. Is this okay for
> trunk?
I think you miss a && DECL_BIT_FIELD_TYPE (TREE_OPERAND (lhs, 1))
after the COMPONENT_REF check. Also while this change is certainly
correct it might be not complete dependent on how the code computes
the access offset - for the C++ memory model the access needs to
be constrained to the DECL_BIT_FIELD_REPRESENTATIVE extent. That is,
a C++ memory model fix would be to supply the bitregion_start and
bitregion_end arguments to get_best_mode. RTL expansion uses
expr.c:get_bit_range as a helper for this. I guess the best thing
would be to export it and re-use it here.
Disclaimer: I think this "optimization" does not belong in
fold-const.c and is very premature at the time we invoke it
(when folding GENERIC from the frontends).
Thanks,
Richard.
>
> Segher
>
>
> 2016-06-10 Segher Boessenkool <seg...@kernel.crashing.org>
>
> PR middle-end/71310
> * fold-const.c (optimize_bit_field_compare): Don't try to use
> word_mode unconditionally for reading the bit field, look at
> DECL_BIT_FIELD_REPRESENTATIVE instead.
>
> ---
> gcc/fold-const.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 5058746..f067001 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -3903,13 +3903,24 @@ optimize_bit_field_compare (location_t loc, enum
> tree_code code,
> return 0;
> }
>
> + /* Don't use a larger mode for reading the bit field than we will
> + use in other places accessing the bit field. */
> + machine_mode largest_mode = word_mode;
> + if (TREE_CODE (lhs) == COMPONENT_REF)
> + {
> + tree field = TREE_OPERAND (lhs, 1);
> + tree repr = DECL_BIT_FIELD_REPRESENTATIVE (field);
> + if (repr)
> + largest_mode = DECL_MODE (repr);
> + }
> +
> /* See if we can find a mode to refer to this field. We should be able to,
> but fail if we can't. */
> nmode = get_best_mode (lbitsize, lbitpos, 0, 0,
> const_p ? TYPE_ALIGN (TREE_TYPE (linner))
> : MIN (TYPE_ALIGN (TREE_TYPE (linner)),
> TYPE_ALIGN (TREE_TYPE (rinner))),
> - word_mode, false);
> + largest_mode, false);
> if (nmode == VOIDmode)
> return 0;
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
