Message ID | alpine.LNX.2.00.1108151436310.810@zhemvz.fhfr.qr |
---|---|
State | New |
Headers | show |
On Aug 15, 2011, at 5:42 AM, Richard Guenther wrote: > The argument still holds that no reasonable memcpy implementation > will reject the src == dest case. Hum... Sounds like if that's the case that we should document it in the manual as something we expect (requirement) of the memcpy implementation. I'll let a frontend or optimization person review this.
On Mon, Aug 15, 2011 at 2:42 PM, Richard Guenther <rguenther@suse.de> wrote: > > The g++.dg/init/copy7.C testcase checks whether the C++ frontend > guards memcpy it emits via a conditional verifying that src != dst > because calling memcpy with overlapping source / destination is > not supported. > > The testcase is misguided though (and the C++ frontend was, until > recently) - the middle-end itself will replace aggregate copies > with memcpy libcalls if it suits - without such conditional. > As PR39480 shows (the bug that prompted to "fixing" the C++ frontend), > the "error" was diagnosed by valgrind, not any real memcpy implemenation. > > The argument still holds that no reasonable memcpy implementation > will reject the src == dest case. Arguing about explicit cache > write-allocation is moot, as you'd still have to handle the > case of memcpy (&a, &a+1, 1) correctly - and thus any reasonable > implementation would handle the src == dest case explicitly if > that is necessary. > > Thus, the following simply removes the now FAILing testcase on > the basis that it never was PASSing really (as my modified > C testcases in PR50079 show). If we ever encounter a platform > that fails for memcpy (&a, &a, ...) and we decide it's not the > platform that is broken we have to invent a fix in the middle-end > and (conditionally) guard any libcall block moves. > > Comments? Ok to commit? Ping? Richard? Jason? Thanks, Richard. > Thanks, > Richard. > > 2011-08-15 Richard Guenther <rguenther@suse.de> > > PR middle-end/50079 > * g++.dg/init/copy7.C: Remove testcase. > > Index: gcc/testsuite/g++.dg/init/copy7.C > =================================================================== > --- gcc/testsuite/g++.dg/init/copy7.C (revision 177759) > +++ gcc/testsuite/g++.dg/init/copy7.C (working copy) > @@ -1,39 +0,0 @@ > -// PR c++/39480 > -// It isn't always safe to call memcpy with identical arguments. > -// { dg-do run } > - > -extern "C" void abort(); > -extern "C" void * > -memcpy(void *dest, void *src, __SIZE_TYPE__ n) > -{ > - if (dest == src) > - abort(); > - else > - { > - __SIZE_TYPE__ i; > - for (i = 0; i < n; i++) > - ((char *)dest)[i] = ((const char*)src)[i]; > - } > -} > - > -struct A > -{ > - double d[10]; > -}; > - > -struct B: public A > -{ > - char bc; > -}; > - > -B b; > - > -void f(B *a1, B* a2) > -{ > - *a1 = *a2; > -} > - > -int main() > -{ > - f(&b,&b); > -} >
On Mon, Oct 10, 2011 at 2:17 PM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Mon, Aug 15, 2011 at 2:42 PM, Richard Guenther <rguenther@suse.de> wrote: >> >> The g++.dg/init/copy7.C testcase checks whether the C++ frontend >> guards memcpy it emits via a conditional verifying that src != dst >> because calling memcpy with overlapping source / destination is >> not supported. >> >> The testcase is misguided though (and the C++ frontend was, until >> recently) - the middle-end itself will replace aggregate copies >> with memcpy libcalls if it suits - without such conditional. >> As PR39480 shows (the bug that prompted to "fixing" the C++ frontend), >> the "error" was diagnosed by valgrind, not any real memcpy implemenation. >> >> The argument still holds that no reasonable memcpy implementation >> will reject the src == dest case. Arguing about explicit cache >> write-allocation is moot, as you'd still have to handle the >> case of memcpy (&a, &a+1, 1) correctly - and thus any reasonable >> implementation would handle the src == dest case explicitly if >> that is necessary. >> >> Thus, the following simply removes the now FAILing testcase on >> the basis that it never was PASSing really (as my modified >> C testcases in PR50079 show). If we ever encounter a platform >> that fails for memcpy (&a, &a, ...) and we decide it's not the >> platform that is broken we have to invent a fix in the middle-end >> and (conditionally) guard any libcall block moves. >> >> Comments? Ok to commit? > > Ping? Richard? Jason? Well, so I went ahead and committed the testsuite patch. Richard.
Index: gcc/testsuite/g++.dg/init/copy7.C =================================================================== --- gcc/testsuite/g++.dg/init/copy7.C (revision 177759) +++ gcc/testsuite/g++.dg/init/copy7.C (working copy) @@ -1,39 +0,0 @@ -// PR c++/39480 -// It isn't always safe to call memcpy with identical arguments. -// { dg-do run } - -extern "C" void abort(); -extern "C" void * -memcpy(void *dest, void *src, __SIZE_TYPE__ n) -{ - if (dest == src) - abort(); - else - { - __SIZE_TYPE__ i; - for (i = 0; i < n; i++) - ((char *)dest)[i] = ((const char*)src)[i]; - } -} - -struct A -{ - double d[10]; -}; - -struct B: public A -{ - char bc; -}; - -B b; - -void f(B *a1, B* a2) -{ - *a1 = *a2; -} - -int main() -{ - f(&b,&b); -}