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

[Martin Liška]

Pinging Eric.

On 11/23/2016 11:49 AM, Richard Biener wrote:
> 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