Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Nov 23, 2016 at 11:26 AM, Martin Liška <mliska@suse.cz> wrote:
> Following patch fixes situation where we do a store to a bitfield which
> is at boundary of a record. This leads to usage of wider store, leading
> to overwriting a following memory location.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> Apart from that, the reported test-case in PR works on x86_64-linux-gnu.
>
> Ready to be installed?

+  HOST_WIDE_INT bitregion_end
+    = exp_size == -1 ? 0 : exp_size * BITS_PER_UNIT - 1;

I don't think looking at the CONSTRUCTOR to determine bitregion_end is
a good idea.
The function gets 'size' as argument which is documented as "number of
bytes we are
allowed to modify" - so better use that.

@@ -6308,7 +6314,8 @@ store_constructor (tree exp, rtx target, int
cleared, HOST_WIDE_INT size,
                MEM_KEEP_ALIAS_SET_P (to_rtx) = 1;
              }

-           store_constructor_field (to_rtx, bitsize, bitpos, mode,
+           store_constructor_field (to_rtx, bitsize, bitpos,
+                                    0, bitregion_end, mode,
                                     value, cleared,
                                     get_alias_set (TREE_TYPE (field)),
                                     reverse);

this stores to to_rtx which may be offsetted from target this means in this case
bitregion_end is not conservative enough - you'd need to resort to the
field width
in that case I guess (and for variable field size not specify any end
-- I suppose
the 'size' store_constructor gets might also be "unknown"?).  But maybe all
the non-constant offset / size cases are "dead code" now that we are in GIMPLE?
Note they likely can only appear from Ada code anyway -- CCing Eric.

I suppose a "safe" thing to do would be to give up on the first
variable offset/size
and re-set bitregion_end to zero for this and all following fields.

The other cases look fine to me.

Thanks,
Richard.

> Martin