On Thu, Jun 11, 2020 at 5:24 PM Eric Botcazou <botca...@adacore.com> wrote:
>
> > I am not very good at reasoning about reverse storage order stuff but
> > can it ever happen that this reverse is not the same as racc->reverse?
> >
> > In order for that to happen, we'd have to have an assignment (folded
> > memcpy?) between two aggregates in the original code that, at the same
> > offset within the aggregates, have single field structures with
> > different storage orders. If that has undefined behavior (even for
> > folded memcpy), I guess I am fine with the patch (but I cannot approve
> > it). If not, I'd add a condition that racc->reverse is the same as
> > TYPE_REVERSE_STORAGE_ORDER(lacc->type) to the if guarding all of this.
>
> You cannot do an assignment between aggregates with opposite storage order,
> that's rejected by the compiler (both in C and Ada); generally speaking, such
> types are not compatible, even if they have the same underlying structure.
> We also block SRA when there is a VIEW_CONVERT_EXPR messing with the storage
> order, see calls to storage_order_barrier_p. And folding a memcpy between
> them should be invalid too (see e.g. the ongoing discussion with Richard in an
> earlier thread). So my understanding is that you don't need the condition.
The patch probably fixes only part of the issues with SRA and rev-storage.
I see in build_ref_for_model:
/* The flag will be set on the record type. */
REF_REVERSE_STORAGE_ORDER (t) = 0;
for the case there was a COMPONENT_REF, so this is for the case where
the model did not have that? Based on your assertion above can we
derive TYPE_REVERSE_STORAGE_ORDER from the passed in racc
and thus set the flag correctly in build_ref_for_model instead?
Thanks,
Richard.
> --
> Eric Botcazou
