This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC, PR 80689] Copy small aggregates element-wise


On Mon, Nov 13, 2017 at 2:46 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Mon, 13 Nov 2017, Richard Biener wrote:
>
>> The main concern here is that GIMPLE is not very well defined for
>> aggregate copies and that gimple-fold.c happily optimizes
>> memcpy (&a, &b, sizeof (a)) into a = b;
>
> What you missed to mention is that we then discussed about rectifying this
> situation by defining GIMPLE more precisely :)  Effectively an aggregate
> assignment in GIMPLE (right now) is always defined to be a dumb block
> copy.  We need a way to describe a member-wise copy as well.  That can
> either be a flag on the statement or implicit by the alias type (i.e.
> block copies always need the ref-all alias type, all others would be
> member-wise copies).

Yes.  Note that it's already GENERIC that needs to nail down the difference.

For the folding there's the possibility of using a char[n] type with n being
constant, thus a new array type for each size, or a char[] type with
variable size, using a WITH_SIZE_EXPR on the RHS (but support for
WITH_SIZE_EXPR isn't so good in passes so I'd rather avoid this for
constant sizes).

The chance here is, of course (find the PR, it exists...), that SRA then
decomposes the char[] copy bytewise...

That said, memcpy folding is easy to fix.  The question is of course
what the semantic of VIEW_CONVERTs is (SRA _does_ contain
bail-outs on those).  Like if you have

struct A { short s; int i; } x;
struct B { int i; short s; } y;

void foo ()
{
  x = VIEW_CONVERT <struct A> (y);
}

so can you access padding via view-converting its value?  Ada uses
VIEW_CONVERT punning on structures a _lot_ (probably the reason
for the SRA bailout).

The above assignment would still be going through that aggregate copy
expansion path.

> Then a user-written memcpy can only be rewritten into a member-wise
> assignment when the types contain no padding, and SRA can only look
> through member-wise assignments when it doesn't see all accesses to the
> destination (when it does then it can look also through block copies).
>
> Then this patch can be restricted to the member-wise assignments (which
> still helps imagemagick as the struct in question doesn't contain padding
> IIRC).
>
> That, or we must dumb down SRA quite much, which I don't think would be a
> good idea.
>
> (I'm not sure if your example would be really valid C as it changes the
> dynamic type of a statically typed declaration; but OTOH we shouldn't
> care, as in GIMPLE the example should of course be expressible)

Yeah, we can equally use allocated storage (but our memcpy folding
then won't apply ...).

Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]