Re: [PATCH 2/3] rs6000: Move g++.dg powerpc PR tests to g++.target
Hi!
On Tue, Feb 22, 2022 at 07:56:40PM -0600, Paul A. Clarke wrote:
> On Tue, Feb 22, 2022 at 06:41:45PM -0600, Segher Boessenkool wrote:
> > That said...
> >
> > > -/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> > > -/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> > > +/* { dg-do compile { target lp64 } } */
> > > +/* { dg-skip-if "" { *-*-darwin* } } */
> >
> > That skip-if is most likely cargo cult, and it's not clear why lp64
> > would be needed either (there is no comment what it is needed for, for
> > example).
>
> I can't speak to darwin, nor have an easy way of testing on it.
Same here, of course.
> As for lp64, these tests fail on -m32 with:
> cc1plus: error: '-mcmodel' not supported in this configuration
> - g++.dg/pr65240-1.C
> - g++.dg/pr65240-2.C
> - g++.dg/pr65240-3.C
>
> '-mcmodel' is in the dg-options line for the above tests.
Yes. That means the dg-options should be conditional (using
dg-additional-options is convenient).
Tests in *.dg should be done anywhere where that doesn't require
heroics to do. This is true to a lesser extent elsewhere as well, and
least true in gcc.target -- but even there still true for many tests.
> The rest PASSed. Shall I remove the 'lp64' restriction for those that PASS?
That is a separate change, so should be a separate commit. If it is
obviously safe, please do it, yes. Thanks!
> > > +++ b/gcc/testsuite/g++.target/powerpc/pr85657.C
> > > @@ -1,4 +1,4 @@
> > > -// { dg-do compile { target { powerpc*-*-linux* } } }
> > > +// { dg-do compile { target { *-*-linux* } } }
> >
> > A comment here would help as well. All of that is pre-existing of
> > course.
>
> I'm not sure what such a comment would say. I suspect it was a testing issue
> (only tested on Linux), but I have similar limitations, so I'm also reluctant
> to enable the test for what would be untested (by me) platforms.
It is obvious what it would say: the reason why this is only tested on
Linux, of course! :-)
I know what you are saying of course. If it isn't obviously safe, it is
not for stage 4. And adding more coverage to existing tests is not very
high value, not high priority at all. The biggest advantage of it would
be that people will stop copying from such bad examples!
Segher
Re: [PING^2 PATCH 2/3] rs6000: Move g++.dg powerpc PR tests to g++.target
Ping.
On Tue, Mar 08, 2022 at 02:03:04PM -0600, Paul A. Clarke via Gcc-patches wrote:
> Gentle ping. I am grateful for the initial review, but seek closure on the
> final couple of discussion items. Thanks!
>
> PC
>
> On Tue, Feb 22, 2022 at 07:56:40PM -0600, Paul A. Clarke via Gcc-patches
> wrote:
> > On Tue, Feb 22, 2022 at 06:41:45PM -0600, Segher Boessenkool wrote:
> > > On Mon, Feb 21, 2022 at 03:17:46PM -0600, Paul A. Clarke wrote:
> > > > Also adjust DejaGnu directives, as specifically requiring
> > > > "powerpc*-*-*" is no
> > > > longer required.
> > > >
> > > > 2021-02-21 Paul A. Clarke
> > > >
> > > > gcc/testsuite
> > > > * g++.dg/pr65240.h: Move to g++.target/powerpc.
> > > > * g++.dg/pr93974.C: Likewise.
> > > > * g++.dg/pr65240-1.C: Move to g++.target/powerpc, adjust dg
> > > > directives.
> > > > * g++.dg/pr65240-2.C: Likewise.
> > > > * g++.dg/pr65240-3.C: Likewise.
> > > > * g++.dg/pr65240-4.C: Likewise.
> > > > * g++.dg/pr65242.C: Likewise.
> > > > * g++.dg/pr67211.C: Likewise.
> > > > * g++.dg/pr69667.C: Likewise.
> > > > * g++.dg/pr71294.C: Likewise.
> > > > * g++.dg/pr84264.C: Likewise.
> > > > * g++.dg/pr84279.C: Likewise.
> > > > * g++.dg/pr85657.C: Likewise.
> > >
> > > Okay for trunk. Thanks!
> >
> > Thanks for the review! More below...
> >
> > > That said...
> > >
> > > > -/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> > > > -/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> > > > +/* { dg-do compile { target lp64 } } */
> > > > +/* { dg-skip-if "" { *-*-darwin* } } */
> > >
> > > That skip-if is most likely cargo cult, and it's not clear why lp64
> > > would be needed either (there is no comment what it is needed for, for
> > > example).
> >
> > I can't speak to darwin, nor have an easy way of testing on it.
> >
> > As for lp64, these tests fail on -m32 with:
> > cc1plus: error: '-mcmodel' not supported in this configuration
> > - g++.dg/pr65240-1.C
> > - g++.dg/pr65240-2.C
> > - g++.dg/pr65240-3.C
> >
> > '-mcmodel' is in the dg-options line for the above tests.
> >
> > The rest PASSed. Shall I remove the 'lp64' restriction for those that PASS?
> >
> > > > +++ b/gcc/testsuite/g++.target/powerpc/pr85657.C
> > > > @@ -1,4 +1,4 @@
> > > > -// { dg-do compile { target { powerpc*-*-linux* } } }
> > > > +// { dg-do compile { target { *-*-linux* } } }
> > >
> > > A comment here would help as well. All of that is pre-existing of
> > > course.
> >
> > I'm not sure what such a comment would say. I suspect it was a testing issue
> > (only tested on Linux), but I have similar limitations, so I'm also
> > reluctant
> > to enable the test for what would be untested (by me) platforms.
> >
> > PC
Re: [PING PATCH 2/3] rs6000: Move g++.dg powerpc PR tests to g++.target
Gentle ping. I am grateful for the initial review, but seek closure on the
final couple of discussion items. Thanks!
PC
On Tue, Feb 22, 2022 at 07:56:40PM -0600, Paul A. Clarke via Gcc-patches wrote:
> On Tue, Feb 22, 2022 at 06:41:45PM -0600, Segher Boessenkool wrote:
> > On Mon, Feb 21, 2022 at 03:17:46PM -0600, Paul A. Clarke wrote:
> > > Also adjust DejaGnu directives, as specifically requiring "powerpc*-*-*"
> > > is no
> > > longer required.
> > >
> > > 2021-02-21 Paul A. Clarke
> > >
> > > gcc/testsuite
> > > * g++.dg/pr65240.h: Move to g++.target/powerpc.
> > > * g++.dg/pr93974.C: Likewise.
> > > * g++.dg/pr65240-1.C: Move to g++.target/powerpc, adjust dg directives.
> > > * g++.dg/pr65240-2.C: Likewise.
> > > * g++.dg/pr65240-3.C: Likewise.
> > > * g++.dg/pr65240-4.C: Likewise.
> > > * g++.dg/pr65242.C: Likewise.
> > > * g++.dg/pr67211.C: Likewise.
> > > * g++.dg/pr69667.C: Likewise.
> > > * g++.dg/pr71294.C: Likewise.
> > > * g++.dg/pr84264.C: Likewise.
> > > * g++.dg/pr84279.C: Likewise.
> > > * g++.dg/pr85657.C: Likewise.
> >
> > Okay for trunk. Thanks!
>
> Thanks for the review! More below...
>
> > That said...
> >
> > > -/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> > > -/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> > > +/* { dg-do compile { target lp64 } } */
> > > +/* { dg-skip-if "" { *-*-darwin* } } */
> >
> > That skip-if is most likely cargo cult, and it's not clear why lp64
> > would be needed either (there is no comment what it is needed for, for
> > example).
>
> I can't speak to darwin, nor have an easy way of testing on it.
>
> As for lp64, these tests fail on -m32 with:
> cc1plus: error: '-mcmodel' not supported in this configuration
> - g++.dg/pr65240-1.C
> - g++.dg/pr65240-2.C
> - g++.dg/pr65240-3.C
>
> '-mcmodel' is in the dg-options line for the above tests.
>
> The rest PASSed. Shall I remove the 'lp64' restriction for those that PASS?
>
> > > +++ b/gcc/testsuite/g++.target/powerpc/pr85657.C
> > > @@ -1,4 +1,4 @@
> > > -// { dg-do compile { target { powerpc*-*-linux* } } }
> > > +// { dg-do compile { target { *-*-linux* } } }
> >
> > A comment here would help as well. All of that is pre-existing of
> > course.
>
> I'm not sure what such a comment would say. I suspect it was a testing issue
> (only tested on Linux), but I have similar limitations, so I'm also reluctant
> to enable the test for what would be untested (by me) platforms.
>
> PC
Re: [PATCH 2/3] rs6000: Move g++.dg powerpc PR tests to g++.target
On Tue, Feb 22, 2022 at 06:41:45PM -0600, Segher Boessenkool wrote:
> On Mon, Feb 21, 2022 at 03:17:46PM -0600, Paul A. Clarke wrote:
> > Also adjust DejaGnu directives, as specifically requiring "powerpc*-*-*" is
> > no
> > longer required.
> >
> > 2021-02-21 Paul A. Clarke
> >
> > gcc/testsuite
> > * g++.dg/pr65240.h: Move to g++.target/powerpc.
> > * g++.dg/pr93974.C: Likewise.
> > * g++.dg/pr65240-1.C: Move to g++.target/powerpc, adjust dg directives.
> > * g++.dg/pr65240-2.C: Likewise.
> > * g++.dg/pr65240-3.C: Likewise.
> > * g++.dg/pr65240-4.C: Likewise.
> > * g++.dg/pr65242.C: Likewise.
> > * g++.dg/pr67211.C: Likewise.
> > * g++.dg/pr69667.C: Likewise.
> > * g++.dg/pr71294.C: Likewise.
> > * g++.dg/pr84264.C: Likewise.
> > * g++.dg/pr84279.C: Likewise.
> > * g++.dg/pr85657.C: Likewise.
>
> Okay for trunk. Thanks!
Thanks for the review! More below...
> That said...
>
> > -/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> > -/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> > +/* { dg-do compile { target lp64 } } */
> > +/* { dg-skip-if "" { *-*-darwin* } } */
>
> That skip-if is most likely cargo cult, and it's not clear why lp64
> would be needed either (there is no comment what it is needed for, for
> example).
I can't speak to darwin, nor have an easy way of testing on it.
As for lp64, these tests fail on -m32 with:
cc1plus: error: '-mcmodel' not supported in this configuration
- g++.dg/pr65240-1.C
- g++.dg/pr65240-2.C
- g++.dg/pr65240-3.C
'-mcmodel' is in the dg-options line for the above tests.
The rest PASSed. Shall I remove the 'lp64' restriction for those that PASS?
> > +++ b/gcc/testsuite/g++.target/powerpc/pr85657.C
> > @@ -1,4 +1,4 @@
> > -// { dg-do compile { target { powerpc*-*-linux* } } }
> > +// { dg-do compile { target { *-*-linux* } } }
>
> A comment here would help as well. All of that is pre-existing of
> course.
I'm not sure what such a comment would say. I suspect it was a testing issue
(only tested on Linux), but I have similar limitations, so I'm also reluctant
to enable the test for what would be untested (by me) platforms.
PC
Re: [PATCH 2/3] rs6000: Move g++.dg powerpc PR tests to g++.target
On Mon, Feb 21, 2022 at 03:17:46PM -0600, Paul A. Clarke wrote:
> Also adjust DejaGnu directives, as specifically requiring "powerpc*-*-*" is no
> longer required.
>
> 2021-02-21 Paul A. Clarke
>
> gcc/testsuite
> * g++.dg/pr65240.h: Move to g++.target/powerpc.
> * g++.dg/pr93974.C: Likewise.
> * g++.dg/pr65240-1.C: Move to g++.target/powerpc, adjust dg directives.
> * g++.dg/pr65240-2.C: Likewise.
> * g++.dg/pr65240-3.C: Likewise.
> * g++.dg/pr65240-4.C: Likewise.
> * g++.dg/pr65242.C: Likewise.
> * g++.dg/pr67211.C: Likewise.
> * g++.dg/pr69667.C: Likewise.
> * g++.dg/pr71294.C: Likewise.
> * g++.dg/pr84264.C: Likewise.
> * g++.dg/pr84279.C: Likewise.
> * g++.dg/pr85657.C: Likewise.
Okay for trunk. Thanks!
That said...
> -/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> -/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> +/* { dg-do compile { target lp64 } } */
> +/* { dg-skip-if "" { *-*-darwin* } } */
That skip-if is most likely cargo cult, and it's not clear why lp64
would be needed either (there is no comment what it is needed for, for
example).
> --- a/gcc/testsuite/g++.dg/pr85657.C
> +++ b/gcc/testsuite/g++.target/powerpc/pr85657.C
> @@ -1,4 +1,4 @@
> -// { dg-do compile { target { powerpc*-*-linux* } } }
> +// { dg-do compile { target { *-*-linux* } } }
A comment here would help as well. All of that is pre-existing of
course.
Segher
[PATCH 2/3] rs6000: Move g++.dg powerpc PR tests to g++.target
Also adjust DejaGnu directives, as specifically requiring "powerpc*-*-*" is no
longer required.
2021-02-21 Paul A. Clarke
gcc/testsuite
* g++.dg/pr65240.h: Move to g++.target/powerpc.
* g++.dg/pr93974.C: Likewise.
* g++.dg/pr65240-1.C: Move to g++.target/powerpc, adjust dg directives.
* g++.dg/pr65240-2.C: Likewise.
* g++.dg/pr65240-3.C: Likewise.
* g++.dg/pr65240-4.C: Likewise.
* g++.dg/pr65242.C: Likewise.
* g++.dg/pr67211.C: Likewise.
* g++.dg/pr69667.C: Likewise.
* g++.dg/pr71294.C: Likewise.
* g++.dg/pr84264.C: Likewise.
* g++.dg/pr84279.C: Likewise.
* g++.dg/pr85657.C: Likewise.
---
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-1.C | 4 ++--
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-2.C | 4 ++--
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-3.C | 4 ++--
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-4.C | 4 ++--
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240.h | 0
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65242.C | 4 ++--
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr67211.C | 4 ++--
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr69667.C | 4 ++--
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr71294.C | 2 +-
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr84264.C | 2 +-
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr84279.C | 4 ++--
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr85657.C | 2 +-
gcc/testsuite/{g++.dg => g++.target/powerpc}/pr93974.C | 0
13 files changed, 19 insertions(+), 19 deletions(-)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-1.C (76%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-2.C (76%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-3.C (76%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240-4.C (75%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65240.h (100%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr65242.C (94%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr67211.C (92%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr69667.C (97%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr71294.C (96%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr84264.C (79%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr84279.C (91%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr85657.C (90%)
rename gcc/testsuite/{g++.dg => g++.target/powerpc}/pr93974.C (100%)
diff --git a/gcc/testsuite/g++.dg/pr65240-1.C
b/gcc/testsuite/g++.target/powerpc/pr65240-1.C
similarity index 76%
rename from gcc/testsuite/g++.dg/pr65240-1.C
rename to gcc/testsuite/g++.target/powerpc/pr65240-1.C
index d2e25b65fcae..d2f4a229773e 100644
--- a/gcc/testsuite/g++.dg/pr65240-1.C
+++ b/gcc/testsuite/g++.target/powerpc/pr65240-1.C
@@ -1,5 +1,5 @@
-/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
-/* { dg-skip-if "" { powerpc*-*-darwin* } } */
+/* { dg-do compile { target lp64 } } */
+/* { dg-skip-if "" { *-*-darwin* } } */
/* { dg-require-effective-target powerpc_p8vector_ok } */
/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power8" } } */
/* { dg-options "-mcpu=power8 -O3 -ffast-math -mcmodel=small -mno-fp-in-toc
-Wno-return-type" } */
diff --git a/gcc/testsuite/g++.dg/pr65240-2.C
b/gcc/testsuite/g++.target/powerpc/pr65240-2.C
similarity index 76%
rename from gcc/testsuite/g++.dg/pr65240-2.C
rename to gcc/testsuite/g++.target/powerpc/pr65240-2.C
index 38d5020bd198..12e36994d27b 100644
--- a/gcc/testsuite/g++.dg/pr65240-2.C
+++ b/gcc/testsuite/g++.target/powerpc/pr65240-2.C
@@ -1,5 +1,5 @@
-/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
-/* { dg-skip-if "" { powerpc*-*-darwin* } } */
+/* { dg-do compile { target lp64 } } */
+/* { dg-skip-if "" { *-*-darwin* } } */
/* { dg-require-effective-target powerpc_p8vector_ok } */
/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power8" } } */
/* { dg-options "-mcpu=power8 -O3 -ffast-math -mcmodel=small -mfp-in-toc
-Wno-return-type" } */
diff --git a/gcc/testsuite/g++.dg/pr65240-3.C
b/gcc/testsuite/g++.target/powerpc/pr65240-3.C
similarity index 76%
rename from gcc/testsuite/g++.dg/pr65240-3.C
rename to gcc/testsuite/g++.target/powerpc/pr65240-3.C
index e8463c914946..9ded3e3ab1d3 100644
--- a/gcc/testsuite/g++.dg/pr65240-3.C
+++ b/gcc/testsuite/g++.target/powerpc/pr65240-3.C
@@ -1,5 +1,5 @@
-/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
-/* { dg-skip-if "" { powerpc*-*-darwin* } } */
+/* { dg-do compile { target lp64 } } */
+/* { dg-skip-if "" { *-*-darwin* } } */
/* { dg-require-effective-target powerpc_p8vector_ok } */
/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power8" } } */
/* { dg-options "-mcpu=power8 -O3 -ffast-math -mcmodel=medium
-Wno-return-type" } */
diff --git a/gcc/testsuite/g++.dg/pr65240-4.C
