On Mon, May 25, 2020 at 12:33 PM Eric Botcazou <botca...@adacore.com> wrote:
>
> Hi,
>
> the attached Ada testcase triggers a GIMPLE verification failure at -O2 or
> above because the GIMPLE store merging pass generates a NOP_EXPR between a FP
> type and an integral type. This happens when the bit-field insertion path is
> taken for a FP field, which can happen in Ada for bit-packed record types.
>
> It is fixed by generating an intermediate VIEW_CONVERT_EXPR. The patch also
> tames a little the bit-field insertion path because, for bit-packed record
> types in Ada, you can end up with large bit-field regions, which results in a
> lot of mask-and-shifts instructions.
>
> Tested on x86-64/Linux, OK for the mainline?
Hmm, MAX_BITSIZE_MODE_ANY_INT looks a bit arbitrary since on
x86 it (IIRC) includes things like OImode. Maybe MOVE_MAX or
MAX_FIXED_MODE_SIZE are better suited here?
Of course the size of the whole region isn't what matters but how
many (if more than one) "region piece" (each word_mode size?)
a store crosses? That is, do we want to prevent store-merging
across such boundaries? Ah, that's the
@@ -4788,7 +4800,7 @@ pass_store_merging::process_store (gimple *stmt)
&& bitsize.is_constant (&const_bitsize)
&& ((const_bitsize % BITS_PER_UNIT) != 0
|| !multiple_p (bitpos, BITS_PER_UNIT))
- && const_bitsize <= 64)
+ && const_bitsize <= MAX_BITSIZE_MODE_ANY_INT)
{
hunk to which I have similar concerns.
The IL correctness fix looks OK to me but it smells like we
might have issues with inserting into x86 XFmode fields
where ordinary XFmode stores store less bytes than
an integer mode of the same size? That issue should be
latent of course (unless it always ICEd before). Also
it's likely reproducible only on Ada(?).
Thanks,
Richard.
>
> 2020-05-25 Eric Botcazou <ebotca...@adacore.com>
>
> * gimple-ssa-store-merging.c (merged_store_group::can_be_merged_into):
> Only turn MEM_REFs into bit-field stores for small bit-field regions.
> (imm_store_chain_info::output_merged_store): Be prepared for sources
> with non-integral type in the bit-field insertion case.
> (pass_store_merging::process_store): Use MAX_BITSIZE_MODE_ANY_INT as
> the largest size for the bit-field case.
>
>
> 2020-05-25 Eric Botcazou <ebotca...@adacore.com>
>
> * gnat.dg/opt84.adb: New test.
>
> --
> Eric Botcazou
