Re: [patch 4/6] scalar-storage-order merge: bulk

[Eric Botcazou <ebotcazou@adacore.com>]

> I suspect there are many comments that we should update as a result of
> this work.  Essentially there's many throughout GCC of the form "On a
> big-endian target" and the like.  With these changes it's more a
> property of the data -- the vast majority of the time the data's
> property is the same as the target, but with this patch it can vary.
> 
> I just happened to spot one in varasm.c::output_constructor_bitfield, as
> the comment started shortly after some code this patch changes.  No
> doubt there's others.  I'm torn whether or not to ask you to scan and
> update them.  It's a lot of work and I'm not terribly sure how valuable
> it'll generally be.

I have adjusted the output_constructor_bitfield case to "For big-endian data".
The cases in expmed.c have naked "big-endian" or "big-endian case" so are OK.
I have changed 2 sibling cases in expr.c but left the others unchanged since 
they are about calling conventions.  I think that's pretty much it for the 
files that are touched by the patch.

> Thanks for not trying too hard to optimize this stuff, it makes the
> expmed.c patches (for example) a lot easier to work through :-)
> 
> I didn't even try to verify you've got all the paths covered.  I mostly
> tried to make sure the patch didn't break existing code.  I'm going to
> assume that the patch essentially works and that if there's paths
> missing where reversal is needed that you'll take care of them as
> they're exposed.

Yes, the main issue in expr.c & expmed.c is getting all the paths covered.
I have run the compile and execute C torture testsuites with -fsso-struct set 
to big-endian on x86-64 so I'm relatively confident, but that's not a proof.
And one of the design constraints of the implementation was to change nothing 
to the default endianness support, so I'm more confident about the absence of 
breakages (IIRC we didn't get a single PR for a breakage with the AdaCore 
compilers while we did get a few for cases where the data was not reversed).

> I'm a bit dismayed at the number of changes to fold-const.c, but
> presumably there's no good way around them.  I probably would have
> looked for a way to punt earlier (that may not be trivial though).
> Given you've done the work, no need to undo it now.

Yes, that's the bitfield optimization stuff, so it depends on the endianness.
Initially the changes were even more pervasive because I tried to optimize 
e.g. equality comparisons of 2 bitfields with reverse endianness in there.
But this broke the "all accesses to a scalar in memory are done with the same 
storage order" invariant and was thus causing annoying issues later on.

> I think this patch is fine.

OK, thanks for the thorough review.  All the parts have been approved, modulo 
the C++ FE part.  As I already explained, this part is very likely incomplete 
(based on the changes that were made in the Ada FE proper) and only makes sure 
that C code compiled by the C++ compiler works as intended, but nothing more.
So I can propose to install only the generic, C and Ada changes for now and to 
explicitly disable the feature for C++ until a more complete implementation of 
the C++ FE part is written.  I'm OK to work on it but I'll probably need help.

-- 
Eric Botcazou