diff mbox

Remove "bogus" g++.dg/init/copy7.C testcase

Message ID alpine.LNX.2.00.1108151436310.810@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Aug. 15, 2011, 12:42 p.m. UTC
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?

Thanks,
Richard.

2011-08-15  Richard Guenther  <rguenther@suse.de>

	PR middle-end/50079
	* g++.dg/init/copy7.C: Remove testcase.

Comments

Mike Stump Aug. 15, 2011, 4:44 p.m. UTC | #1
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.
Richard Biener Oct. 10, 2011, 12:17 p.m. UTC | #2
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);
> -}
>
Richard Biener Nov. 3, 2011, 3:15 p.m. UTC | #3
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.
diff mbox

Patch

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);
-}