Re: [PATCH] Turn some compile-time tests into run-time tests

[Jeff Law]

On 04/28/2016 08:03 AM, Patrick Palka wrote:



The rest seem OK to me.  Note that I'm not convinced all these tests were
designed to be execution tests, even though they use __builtin_abort and
friends.  Though it's a good marker of something that can/should be looked
at.


True..  What made me look into this in the first place is that I
caught myself making a similar mistake, i.e. marking an execution test
case as dg-do compile instead of dg-do run out of habit.
It's an easy mistake to make and, it's pretty low in terms of real world 
impact :-)



 But I

suppose it's worth looking at the context of each of these tests to
see if they were not actually intended to be execution tests.  I'll
double check this and report back; in the meantime I also found some
more tests that ought to be looked at.
I think for the set you already identified go ahead and make the 
approved changes.  We don't really lose anything by doing so.  Going 
forward we just have to continue to watch for this kind of thing 
slipping through the cracks and updating tests as mistakes are identified.


jeff

Re: [PATCH] Turn some compile-time tests into run-time tests

[Patrick Palka]

On Wed, Apr 27, 2016 at 5:36 PM, Jeff Law  wrote:
> On 03/10/2016 04:38 PM, Patrick Palka wrote:
>>
>> I ran the command
>>
>>   git grep -l "dg-do compile" | xargs grep -l __builtin_abort | xargs grep
>> -lw main
>>
>> to find tests marked as compile-time tests that likely ought to instead
>> be marked as run-time tests, by the rationale that they use
>> __builtin_abort and they also define main().  (I also then confirmed that
>> they
>> compile, link and run cleanly on my machine.)
>>
>> After this patch, the remaining test files reported by the above command
>> are:
>>
>>   These do not define all the functions they use:
>> gcc/testsuite/g++.dg/ipa/devirt-41.C
>> gcc/testsuite/g++.dg/ipa/devirt-44.C
>> gcc/testsuite/g++.dg/ipa/devirt-45.C
>> gcc/testsuite/gcc.target/i386/pr55672.c
>>
>>   These are non-x86 tests so I can't confirm that they run cleanly:
>> gcc/testsuite/gcc.target/arm/pr58041.c
>> gcc/testsuite/gcc.target/powerpc/pr35907.c
>> gcc/testsuite/gcc.target/s390/dwarfregtable-1.c
>> gcc/testsuite/gcc.target/s390/dwarfregtable-2.c
>> gcc/testsuite/gcc.target/s390/dwarfregtable-3.c
>>
>>   These use dg-error:
>> libstdc++-v3/testsuite/20_util/forward/c_neg.cc
>> libstdc++-v3/testsuite/20_util/forward/f_neg.cc
>>
>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
>> commit?  Does anyone have another heuristic one can use to help find
>> these kinds of typos?
>>
>> gcc/testsuite/ChangeLog:
>>
>> * g++.dg/cpp0x/constexpr-aggr2.C: Make it a run-time test.
>> * g++.dg/cpp0x/nullptr32.C: Likewise.
>> * g++.dg/cpp1y/digit-sep-cxx11-neg.C: Likewise.
>> * g++.dg/cpp1y/digit-sep.C: Likewise.
>> * g++.dg/ext/flexary13.C: Likewise.
>> * gcc.dg/alias-14.c: Likewise.
>> * gcc.dg/ipa/PR65282.c: Likewise.
>> * gcc.dg/pr69644.c: Likewise.
>> * gcc.dg/tree-ssa/pr38533.c: Likewise.
>> * gcc.dg/tree-ssa/pr61385.c: Likewise.
>
> My worry with the 38533 test is that while the ASM defines "f" from the
> standpoint of dataflow, it does not actually emit any code to ensure "f" is
> actually defined.  This could lead to spurious aborts due to use of an
> uninitialized value at runtime.  Similarly for alias-14.c
>
> I'd be worried that we don't necessarily have sync_bool_compare_and_swap on
> all targets for 69644.

Ah yeah, good points..

>
> flexary13.C probably won't link on a cross target unless the cross libraries
> are available.  But that's probably OK.
>
> The rest seem OK to me.  Note that I'm not convinced all these tests were
> designed to be execution tests, even though they use __builtin_abort and
> friends.  Though it's a good marker of something that can/should be looked
> at.

True..  What made me look into this in the first place is that I
caught myself making a similar mistake, i.e. marking an execution test
case as dg-do compile instead of dg-do run out of habit.  But I
suppose it's worth looking at the context of each of these tests to
see if they were not actually intended to be execution tests.  I'll
double check this and report back; in the meantime I also found some
more tests that ought to be looked at.

>
>
> jeff
>

Re: [PATCH] Turn some compile-time tests into run-time tests

[Jeff Law]

On 03/10/2016 04:38 PM, Patrick Palka wrote:

I ran the command

  git grep -l "dg-do compile" | xargs grep -l __builtin_abort | xargs grep -lw 
main

to find tests marked as compile-time tests that likely ought to instead
be marked as run-time tests, by the rationale that they use
__builtin_abort and they also define main().  (I also then confirmed that they
compile, link and run cleanly on my machine.)

After this patch, the remaining test files reported by the above command
are:

  These do not define all the functions they use:
gcc/testsuite/g++.dg/ipa/devirt-41.C
gcc/testsuite/g++.dg/ipa/devirt-44.C
gcc/testsuite/g++.dg/ipa/devirt-45.C
gcc/testsuite/gcc.target/i386/pr55672.c

  These are non-x86 tests so I can't confirm that they run cleanly:
gcc/testsuite/gcc.target/arm/pr58041.c
gcc/testsuite/gcc.target/powerpc/pr35907.c
gcc/testsuite/gcc.target/s390/dwarfregtable-1.c
gcc/testsuite/gcc.target/s390/dwarfregtable-2.c
gcc/testsuite/gcc.target/s390/dwarfregtable-3.c

  These use dg-error:
libstdc++-v3/testsuite/20_util/forward/c_neg.cc
libstdc++-v3/testsuite/20_util/forward/f_neg.cc

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
commit?  Does anyone have another heuristic one can use to help find
these kinds of typos?

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/constexpr-aggr2.C: Make it a run-time test.
* g++.dg/cpp0x/nullptr32.C: Likewise.
* g++.dg/cpp1y/digit-sep-cxx11-neg.C: Likewise.
* g++.dg/cpp1y/digit-sep.C: Likewise.
* g++.dg/ext/flexary13.C: Likewise.
* gcc.dg/alias-14.c: Likewise.
* gcc.dg/ipa/PR65282.c: Likewise.
* gcc.dg/pr69644.c: Likewise.
* gcc.dg/tree-ssa/pr38533.c: Likewise.
* gcc.dg/tree-ssa/pr61385.c: Likewise.
My worry with the 38533 test is that while the ASM defines "f" from the 
standpoint of dataflow, it does not actually emit any code to ensure "f" 
is actually defined.  This could lead to spurious aborts due to use of 
an uninitialized value at runtime.  Similarly for alias-14.c


I'd be worried that we don't necessarily have sync_bool_compare_and_swap 
on all targets for 69644.


flexary13.C probably won't link on a cross target unless the cross 
libraries are available.  But that's probably OK.


The rest seem OK to me.  Note that I'm not convinced all these tests 
were designed to be execution tests, even though they use 
__builtin_abort and friends.  Though it's a good marker of something 
that can/should be looked at.



jeff

Re: [PATCH] Turn some compile-time tests into run-time tests

[Jeff Law]

On 03/11/2016 09:38 AM, Patrick Palka wrote:

On Thu, Mar 10, 2016 at 6:38 PM, Patrick Palka  wrote:

I ran the command

  git grep -l "dg-do compile" | xargs grep -l __builtin_abort | xargs grep -lw 
main

to find tests marked as compile-time tests that likely ought to instead
be marked as run-time tests, by the rationale that they use
__builtin_abort and they also define main().  (I also then confirmed that they
compile, link and run cleanly on my machine.)

After this patch, the remaining test files reported by the above command
are:

  These do not define all the functions they use:
gcc/testsuite/g++.dg/ipa/devirt-41.C
gcc/testsuite/g++.dg/ipa/devirt-44.C
gcc/testsuite/g++.dg/ipa/devirt-45.C
gcc/testsuite/gcc.target/i386/pr55672.c

  These are non-x86 tests so I can't confirm that they run cleanly:
gcc/testsuite/gcc.target/arm/pr58041.c
gcc/testsuite/gcc.target/powerpc/pr35907.c
gcc/testsuite/gcc.target/s390/dwarfregtable-1.c
gcc/testsuite/gcc.target/s390/dwarfregtable-2.c
gcc/testsuite/gcc.target/s390/dwarfregtable-3.c

  These use dg-error:
libstdc++-v3/testsuite/20_util/forward/c_neg.cc
libstdc++-v3/testsuite/20_util/forward/f_neg.cc

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
commit?  Does anyone have another heuristic one can use to help find
these kinds of typos?

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/constexpr-aggr2.C: Make it a run-time test.
* g++.dg/cpp0x/nullptr32.C: Likewise.
* g++.dg/cpp1y/digit-sep-cxx11-neg.C: Likewise.
* g++.dg/cpp1y/digit-sep.C: Likewise.
* g++.dg/ext/flexary13.C: Likewise.
* gcc.dg/alias-14.c: Likewise.
* gcc.dg/ipa/PR65282.c: Likewise.
* gcc.dg/pr69644.c: Likewise.
* gcc.dg/tree-ssa/pr38533.c: Likewise.
* gcc.dg/tree-ssa/pr61385.c: Likewise.


Here's another I found:

diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
index 4b353b6..ea7ae6f 100644
--- a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
THe lambda-return1.C change is fine with an appropriate ChangeLog.  I'll 
be looking at the others momentarily.


jeff

Re: [PATCH] Turn some compile-time tests into run-time tests

[Patrick Palka]

On Thu, Mar 10, 2016 at 6:38 PM, Patrick Palka  wrote:
> I ran the command
>
>   git grep -l "dg-do compile" | xargs grep -l __builtin_abort | xargs grep 
> -lw main
>
> to find tests marked as compile-time tests that likely ought to instead
> be marked as run-time tests, by the rationale that they use
> __builtin_abort and they also define main().  (I also then confirmed that they
> compile, link and run cleanly on my machine.)
>
> After this patch, the remaining test files reported by the above command
> are:
>
>   These do not define all the functions they use:
> gcc/testsuite/g++.dg/ipa/devirt-41.C
> gcc/testsuite/g++.dg/ipa/devirt-44.C
> gcc/testsuite/g++.dg/ipa/devirt-45.C
> gcc/testsuite/gcc.target/i386/pr55672.c
>
>   These are non-x86 tests so I can't confirm that they run cleanly:
> gcc/testsuite/gcc.target/arm/pr58041.c
> gcc/testsuite/gcc.target/powerpc/pr35907.c
> gcc/testsuite/gcc.target/s390/dwarfregtable-1.c
> gcc/testsuite/gcc.target/s390/dwarfregtable-2.c
> gcc/testsuite/gcc.target/s390/dwarfregtable-3.c
>
>   These use dg-error:
> libstdc++-v3/testsuite/20_util/forward/c_neg.cc
> libstdc++-v3/testsuite/20_util/forward/f_neg.cc
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
> commit?  Does anyone have another heuristic one can use to help find
> these kinds of typos?
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/constexpr-aggr2.C: Make it a run-time test.
> * g++.dg/cpp0x/nullptr32.C: Likewise.
> * g++.dg/cpp1y/digit-sep-cxx11-neg.C: Likewise.
> * g++.dg/cpp1y/digit-sep.C: Likewise.
> * g++.dg/ext/flexary13.C: Likewise.
> * gcc.dg/alias-14.c: Likewise.
> * gcc.dg/ipa/PR65282.c: Likewise.
> * gcc.dg/pr69644.c: Likewise.
> * gcc.dg/tree-ssa/pr38533.c: Likewise.
> * gcc.dg/tree-ssa/pr61385.c: Likewise.

Here's another I found:

diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
index 4b353b6..ea7ae6f 100644
--- a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
@@ -1,5 +1,5 @@
 // PR c++/57437
-// { dg-require-effective-target c++11 }
+// { dg-do run { target c++11 } }

 struct A {
   int i;

Re: [PATCH] Turn some compile-time tests into run-time tests

[Patrick Palka]

On Thu, Mar 10, 2016 at 6:38 PM, Patrick Palka  wrote:
> I ran the command
>
>   git grep -l "dg-do compile" | xargs grep -l __builtin_abort | xargs grep 
> -lw main
>
> to find tests marked as compile-time tests that likely ought to instead
> be marked as run-time tests, by the rationale that they use
> __builtin_abort and they also define main().  (I also then confirmed that they
> compile, link and run cleanly on my machine.)
>
> After this patch, the remaining test files reported by the above command
> are:
>
>   These do not define all the functions they use:
> gcc/testsuite/g++.dg/ipa/devirt-41.C
> gcc/testsuite/g++.dg/ipa/devirt-44.C
> gcc/testsuite/g++.dg/ipa/devirt-45.C
> gcc/testsuite/gcc.target/i386/pr55672.c

Actually this i386 test just performs obvious UB which is why I left
it as a compile-time test.

[PATCH] Turn some compile-time tests into run-time tests

[Patrick Palka]

I ran the command

  git grep -l "dg-do compile" | xargs grep -l __builtin_abort | xargs grep -lw 
main

to find tests marked as compile-time tests that likely ought to instead
be marked as run-time tests, by the rationale that they use
__builtin_abort and they also define main().  (I also then confirmed that they
compile, link and run cleanly on my machine.)

After this patch, the remaining test files reported by the above command
are:

  These do not define all the functions they use:
gcc/testsuite/g++.dg/ipa/devirt-41.C
gcc/testsuite/g++.dg/ipa/devirt-44.C
gcc/testsuite/g++.dg/ipa/devirt-45.C
gcc/testsuite/gcc.target/i386/pr55672.c

  These are non-x86 tests so I can't confirm that they run cleanly:
gcc/testsuite/gcc.target/arm/pr58041.c
gcc/testsuite/gcc.target/powerpc/pr35907.c
gcc/testsuite/gcc.target/s390/dwarfregtable-1.c
gcc/testsuite/gcc.target/s390/dwarfregtable-2.c
gcc/testsuite/gcc.target/s390/dwarfregtable-3.c

  These use dg-error:
libstdc++-v3/testsuite/20_util/forward/c_neg.cc
libstdc++-v3/testsuite/20_util/forward/f_neg.cc

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
commit?  Does anyone have another heuristic one can use to help find
these kinds of typos?

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/constexpr-aggr2.C: Make it a run-time test.
* g++.dg/cpp0x/nullptr32.C: Likewise.
* g++.dg/cpp1y/digit-sep-cxx11-neg.C: Likewise.
* g++.dg/cpp1y/digit-sep.C: Likewise.
* g++.dg/ext/flexary13.C: Likewise.
* gcc.dg/alias-14.c: Likewise.
* gcc.dg/ipa/PR65282.c: Likewise.
* gcc.dg/pr69644.c: Likewise.
* gcc.dg/tree-ssa/pr38533.c: Likewise.
* gcc.dg/tree-ssa/pr61385.c: Likewise.
---
 gcc/testsuite/g++.dg/cpp0x/constexpr-aggr2.C | 2 +-
 gcc/testsuite/g++.dg/cpp0x/nullptr32.C   | 2 +-
 gcc/testsuite/g++.dg/cpp1y/digit-sep-cxx11-neg.C | 2 +-
 gcc/testsuite/g++.dg/cpp1y/digit-sep.C   | 2 +-
 gcc/testsuite/g++.dg/ext/flexary13.C | 2 +-
 gcc/testsuite/gcc.dg/alias-14.c  | 2 +-
 gcc/testsuite/gcc.dg/ipa/PR65282.c   | 2 +-
 gcc/testsuite/gcc.dg/pr69644.c   | 2 +-
 gcc/testsuite/gcc.dg/tree-ssa/pr38533.c  | 2 +-
 gcc/testsuite/gcc.dg/tree-ssa/pr61385.c  | 2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-aggr2.C 
b/gcc/testsuite/g++.dg/cpp0x/constexpr-aggr2.C
index 805d026..ed01aad 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-aggr2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-aggr2.C
@@ -1,5 +1,5 @@
 // PR c++/68782
-// { dg-do compile { target c++11 } }
+// { dg-do run { target c++11 } }
 
 #define assert(X) do { if (!(X)) __builtin_abort(); } while (0)
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr32.C 
b/gcc/testsuite/g++.dg/cpp0x/nullptr32.C
index a1e6710..4de4140 100644
--- a/gcc/testsuite/g++.dg/cpp0x/nullptr32.C
+++ b/gcc/testsuite/g++.dg/cpp0x/nullptr32.C
@@ -1,6 +1,6 @@
 // PR c++/63942
 // A mangling alias for the first constructor was conflicting with the second.
-// { dg-do compile { target c++11 } }
+// { dg-do run { target c++11 } }
 // { dg-options "-fno-inline" }
 
 int i;
diff --git a/gcc/testsuite/g++.dg/cpp1y/digit-sep-cxx11-neg.C 
b/gcc/testsuite/g++.dg/cpp1y/digit-sep-cxx11-neg.C
index 411ba52..c1cbc6c 100644
--- a/gcc/testsuite/g++.dg/cpp1y/digit-sep-cxx11-neg.C
+++ b/gcc/testsuite/g++.dg/cpp1y/digit-sep-cxx11-neg.C
@@ -1,4 +1,4 @@
-// { dg-do compile { target { ! c++14 } } }
+// { dg-do run { target { ! c++14 } } }
 
 #define assert(E) if(!(E))__builtin_abort();
 
diff --git a/gcc/testsuite/g++.dg/cpp1y/digit-sep.C 
b/gcc/testsuite/g++.dg/cpp1y/digit-sep.C
index 111e7a5..75a12f6 100644
--- a/gcc/testsuite/g++.dg/cpp1y/digit-sep.C
+++ b/gcc/testsuite/g++.dg/cpp1y/digit-sep.C
@@ -1,4 +1,4 @@
-// { dg-do compile { target c++14 } }
+// { dg-do run { target c++14 } }
 
 #define assert(E) if(!(E))__builtin_abort();
 
diff --git a/gcc/testsuite/g++.dg/ext/flexary13.C 
b/gcc/testsuite/g++.dg/ext/flexary13.C
index 462ed65..1347542 100644
--- a/gcc/testsuite/g++.dg/ext/flexary13.C
+++ b/gcc/testsuite/g++.dg/ext/flexary13.C
@@ -1,4 +1,4 @@
-// { dg-do compile }
+// { dg-do run }
 // { dg-options -Wno-pedantic }
 
 #define STR(s) #s
diff --git a/gcc/testsuite/gcc.dg/alias-14.c b/gcc/testsuite/gcc.dg/alias-14.c
index 1ca1c09..24f0d1c 100644
--- a/gcc/testsuite/gcc.dg/alias-14.c
+++ b/gcc/testsuite/gcc.dg/alias-14.c
@@ -1,4 +1,4 @@
-/* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O2" } */
 #include 
 void *a;
diff --git a/gcc/testsuite/gcc.dg/ipa/PR65282.c 
b/gcc/testsuite/gcc.dg/ipa/PR65282.c
index a52bec6..a7a881a 100644
--- a/gcc/testsuite/gcc.dg/ipa/PR65282.c
+++ b/gcc/testsuite/gcc.dg/ipa/PR65282.c
@@ -1,4 +1,4 @@
-/* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-Os -fdump-ipa-icf"  } */
 
 int a[2];
diff --git a/gcc/testsuite/gcc.dg/pr69644.c