On Fri, Nov 24, 2017 at 11:31 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Thu, Nov 23, 2017 at 4:32 PM, Martin Jambor <mjam...@suse.cz> wrote:
>> Hi,
>>
>> On Mon, Nov 13 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;
>>>
>>> struct A { short s; long i; long j; };
>>> struct A a, b;
>>> void foo ()
>>> {
>>> __builtin_memcpy (&a, &b, sizeof (struct A));
>>> }
>>>
>>> gets folded to
>>>
>>> MEM[(char * {ref-all})&a] = MEM[(char * {ref-all})&b];
>>> return;
>>>
>>> you see we're careful about TBAA but (don't see that above but
>>> can be verified by for example debugging expand_assignment)
>>> TREE_TYPE (MEM[...]) is actually 'struct A'.
>>>
>>> And yes, I've been worried about SRA as well here... it _does_
>>> have some early outs when seeing VIEW_CONVERT_EXPR but
>>> appearantly not for the above. Testcase that aborts with SRA but
>>> not without:
>>>
>>> struct A { short s; long i; long j; };
>>> struct A a, b;
>>> void foo ()
>>> {
>>> struct A c;
>>> __builtin_memcpy (&c, &b, sizeof (struct A));
>>> __builtin_memcpy (&a, &c, sizeof (struct A));
>>> }
>>> int main()
>>> {
>>> __builtin_memset (&b, 0, sizeof (struct A));
>>> b.s = 1;
>>> __builtin_memcpy ((char *)&b+2, &b, 2);
>>> foo ();
>>> __builtin_memcpy (&a, (char *)&a+2, 2);
>>> if (a.s != 1)
>>> __builtin_abort ();
>>> return 0;
>>> }
>>
>> Thanks for the testcase, I agree that is a fairly big problem. Do you
>> think that the following (untested) patch is an appropriate way of
>> fixing it and generally of extending gimple to capture that a statement
>> is a bit-copy?
>
> I think the place to fix is the memcpy folding. That is, we'd say that
> aggregate assignments are not bit-copies but do element-wise assignments.
> For memcpy folding we'd then need to use a type that doesn't contain
> padding. Which effectively means char[].
>
> Of course we need to stop SRA from decomposing that copy to
> individual characters then ;)
>
> So iff we decide that all aggregate copies are element copies,
> maybe only those where TYPE_MAIN_VARIANT of lhs and rhs match
> (currently we allow TYPE_CANONICAL compatibility and thus there
> might be some mismatches), then we have to fix nothign but
> the memcpy folding.
>
>> If so, I'll add the testcase, bootstrap it and formally propose it.
>> Subsequently I will of course make sure that any element-wise copying
>> patch would test the predicate.
>
> I don't think the alias-set should determine whether a copy is
> bit-wise or not.Like the attached. At least FAILs FAIL: gcc.dg/tree-ssa/ssa-ccp-27.c scan-tree-dump-times ccp1 "memcpy[^\n]*123456" 2 (found 0 times) not sure why we have this test. Richard.
p3-2
Description: Binary data
