| Message ID | 20210330104358.GH1179226@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | testsuite: Disable zero-scratch-regs-{8,9,10,11}.c on s390* [PR97680] | expand |
On Tue, Mar 30, 2021 at 12:44 PM Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Hi! > > These test FAIL on s390*: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c: In function 'foo8': > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c:71:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > compiler exited with status 1 > FAIL: c-c++-common/zero-scratch-regs-10.c -Wc++-compat (test for excess errors) > Excess errors: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c:71:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c: In function 'foo8': > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c:71:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > compiler exited with status 1 > FAIL: c-c++-common/zero-scratch-regs-11.c -Wc++-compat (test for excess errors) > Excess errors: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c:71:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > In file included from /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c:5: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c: In function 'foo': > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c:10:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > compiler exited with status 1 > FAIL: c-c++-common/zero-scratch-regs-8.c -Wc++-compat (test for excess errors) > Excess errors: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c:10:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c: In function 'foo': > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c:10:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > compiler exited with status 1 > FAIL: c-c++-common/zero-scratch-regs-9.c -Wc++-compat (test for excess errors) > Excess errors: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c:10:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > Like on powerpc or arm, they need backend support which isn't there and > likely should be added for GCC 12. > > Ok to skip the test on s390* until then? Can we change the test to do { dg-skip-if "not implemented" { ! { target x86_64-*-* <few others> } } } instead? IIRC it's nowhere implemented but on x86_64. > 2021-03-30 Jakub Jelinek <jakub@redhat.com> > > PR testsuite/97680 > * c-c++-common/zero-scratch-regs-8.c: Skip on s390. > * c-c++-common/zero-scratch-regs-9.c: Likewise. > * c-c++-common/zero-scratch-regs-10.c: Likewise. > * c-c++-common/zero-scratch-regs-11.c: Likewise. > > --- gcc/testsuite/c-c++-common/zero-scratch-regs-8.c.jj 2020-11-11 01:46:03.392696119 +0100 > +++ gcc/testsuite/c-c++-common/zero-scratch-regs-8.c 2021-03-30 12:32:11.099667255 +0200 > @@ -1,5 +1,6 @@ > /* { dg-do run } */ > /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ > +/* { dg-skip-if "not implemented" { s390*-*-* } } */ > /* { dg-options "-O2 -fzero-call-used-regs=all-arg" } */ > > #include "zero-scratch-regs-1.c" > --- gcc/testsuite/c-c++-common/zero-scratch-regs-9.c.jj 2020-11-11 01:46:03.392696119 +0100 > +++ gcc/testsuite/c-c++-common/zero-scratch-regs-9.c 2021-03-30 12:32:26.707493760 +0200 > @@ -1,5 +1,6 @@ > /* { dg-do run } */ > /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ > +/* { dg-skip-if "not implemented" { s390*-*-* } } */ > /* { dg-options "-O2 -fzero-call-used-regs=all" } */ > > #include "zero-scratch-regs-1.c" > --- gcc/testsuite/c-c++-common/zero-scratch-regs-10.c.jj 2021-03-18 15:32:56.459617723 +0100 > +++ gcc/testsuite/c-c++-common/zero-scratch-regs-10.c 2021-03-30 12:31:56.468829910 +0200 > @@ -1,5 +1,6 @@ > /* { dg-do run } */ > /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ > +/* { dg-skip-if "not implemented" { s390*-*-* } } */ > /* { dg-skip-if "not implemented" { arm*-*-* } } */ > /* { dg-options "-O2" } */ > > --- gcc/testsuite/c-c++-common/zero-scratch-regs-11.c.jj 2020-11-11 01:46:03.392696119 +0100 > +++ gcc/testsuite/c-c++-common/zero-scratch-regs-11.c 2021-03-30 12:32:46.012279152 +0200 > @@ -1,5 +1,6 @@ > /* { dg-do run } */ > /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ > +/* { dg-skip-if "not implemented" { s390*-*-* } } */ > /* { dg-options "-O2 -fzero-call-used-regs=all" } */ > > #include "zero-scratch-regs-10.c" > > Jakub >
On Tue, Mar 30, 2021 at 01:28:40PM +0200, Richard Biener wrote: > > Ok to skip the test on s390* until then? > > Can we change the test to do > > { dg-skip-if "not implemented" { ! { target x86_64-*-* <few others> } } } > > instead? IIRC it's nowhere implemented but on x86_64. I don't know, perhaps. Seems the target hook is only defined on config/i386/i386.c:#undef TARGET_ZERO_CALL_USED_REGS config/i386/i386.c:#define TARGET_ZERO_CALL_USED_REGS ix86_zero_call_used_regs config/sparc/sparc.c:#undef TARGET_ZERO_CALL_USED_REGS config/sparc/sparc.c:#define TARGET_ZERO_CALL_USED_REGS sparc_zero_call_used_regs but apparently many of the tests actually succeed on various targets that don't define those hooks. E.g. I haven't seen them to fail on aarch64, on arm only the -10.c fails, on powerpc*/s390* all {8,9,10,11} fail (plus 5 is skipped on power*-aix*). On ia64 according to testresults {6,7,8,9,10,11} fail, some with ICEs. On mipsel according to testresults {9,10,11} fail, some with ICEs. On nvptx at least 1-9 succeed, 10-11 don't know, don't have assert.h around. So, do we want to fill in negative dg-skip-if for the 6-11 tests or positive? In any case, is there any hope any of the maintainers or the original submitter will change anything for GCC 12, or are we going to end up with a very narrowly supported feature? Jakub
On Tue, Mar 30, 2021 at 1:56 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Mar 30, 2021 at 01:28:40PM +0200, Richard Biener wrote: > > > Ok to skip the test on s390* until then? > > > > Can we change the test to do > > > > { dg-skip-if "not implemented" { ! { target x86_64-*-* <few others> } } } > > > > instead? IIRC it's nowhere implemented but on x86_64. > > I don't know, perhaps. > Seems the target hook is only defined on > config/i386/i386.c:#undef TARGET_ZERO_CALL_USED_REGS > config/i386/i386.c:#define TARGET_ZERO_CALL_USED_REGS ix86_zero_call_used_regs > config/sparc/sparc.c:#undef TARGET_ZERO_CALL_USED_REGS > config/sparc/sparc.c:#define TARGET_ZERO_CALL_USED_REGS sparc_zero_call_used_regs > but apparently many of the tests actually succeed on various targets that > don't define those hooks. E.g. I haven't seen them to fail on aarch64, > on arm only the -10.c fails, on powerpc*/s390* all {8,9,10,11} fail (plus > 5 is skipped on power*-aix*). > On ia64 according to testresults {6,7,8,9,10,11} fail, some with ICEs. > On mipsel according to testresults {9,10,11} fail, some with ICEs. > On nvptx at least 1-9 succeed, 10-11 don't know, don't have assert.h around. > > So, do we want to fill in negative dg-skip-if for the 6-11 tests or > positive? In any case, is there any hope any of the maintainers or the > original submitter will change anything for GCC 12, or are we going to end > up with a very narrowly supported feature? It looks like the latter - I've seen no attempt by the original authors to make the feature work on more targets than they cared for. Richard. > Jakub >
On 3/30/21 12:43 PM, Jakub Jelinek wrote: > Hi! > > These test FAIL on s390*: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c: In function 'foo8': > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c:71:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > compiler exited with status 1 > FAIL: c-c++-common/zero-scratch-regs-10.c -Wc++-compat (test for excess errors) > Excess errors: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c:71:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c: In function 'foo8': > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c:71:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > compiler exited with status 1 > FAIL: c-c++-common/zero-scratch-regs-11.c -Wc++-compat (test for excess errors) > Excess errors: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c:71:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > In file included from /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c:5: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c: In function 'foo': > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c:10:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > compiler exited with status 1 > FAIL: c-c++-common/zero-scratch-regs-8.c -Wc++-compat (test for excess errors) > Excess errors: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c:10:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c: In function 'foo': > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c:10:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > compiler exited with status 1 > FAIL: c-c++-common/zero-scratch-regs-9.c -Wc++-compat (test for excess errors) > Excess errors: > /builddir/build/BUILD/gcc-11.0.1-20210324/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c:10:1: sorry, unimplemented: '-fzero-call-used-regs' not supported on this target > Like on powerpc or arm, they need backend support which isn't there and > likely should be added for GCC 12. > > Ok to skip the test on s390* until then? Fine with me. Thanks! Andreas > > 2021-03-30 Jakub Jelinek <jakub@redhat.com> > > PR testsuite/97680 > * c-c++-common/zero-scratch-regs-8.c: Skip on s390. > * c-c++-common/zero-scratch-regs-9.c: Likewise. > * c-c++-common/zero-scratch-regs-10.c: Likewise. > * c-c++-common/zero-scratch-regs-11.c: Likewise. > > --- gcc/testsuite/c-c++-common/zero-scratch-regs-8.c.jj 2020-11-11 01:46:03.392696119 +0100 > +++ gcc/testsuite/c-c++-common/zero-scratch-regs-8.c 2021-03-30 12:32:11.099667255 +0200 > @@ -1,5 +1,6 @@ > /* { dg-do run } */ > /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ > +/* { dg-skip-if "not implemented" { s390*-*-* } } */ > /* { dg-options "-O2 -fzero-call-used-regs=all-arg" } */ > > #include "zero-scratch-regs-1.c" > --- gcc/testsuite/c-c++-common/zero-scratch-regs-9.c.jj 2020-11-11 01:46:03.392696119 +0100 > +++ gcc/testsuite/c-c++-common/zero-scratch-regs-9.c 2021-03-30 12:32:26.707493760 +0200 > @@ -1,5 +1,6 @@ > /* { dg-do run } */ > /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ > +/* { dg-skip-if "not implemented" { s390*-*-* } } */ > /* { dg-options "-O2 -fzero-call-used-regs=all" } */ > > #include "zero-scratch-regs-1.c" > --- gcc/testsuite/c-c++-common/zero-scratch-regs-10.c.jj 2021-03-18 15:32:56.459617723 +0100 > +++ gcc/testsuite/c-c++-common/zero-scratch-regs-10.c 2021-03-30 12:31:56.468829910 +0200 > @@ -1,5 +1,6 @@ > /* { dg-do run } */ > /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ > +/* { dg-skip-if "not implemented" { s390*-*-* } } */ > /* { dg-skip-if "not implemented" { arm*-*-* } } */ > /* { dg-options "-O2" } */ > > --- gcc/testsuite/c-c++-common/zero-scratch-regs-11.c.jj 2020-11-11 01:46:03.392696119 +0100 > +++ gcc/testsuite/c-c++-common/zero-scratch-regs-11.c 2021-03-30 12:32:46.012279152 +0200 > @@ -1,5 +1,6 @@ > /* { dg-do run } */ > /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ > +/* { dg-skip-if "not implemented" { s390*-*-* } } */ > /* { dg-options "-O2 -fzero-call-used-regs=all" } */ > > #include "zero-scratch-regs-10.c" > > Jakub >
> It looks like the latter - I've seen no attempt by the original authors to > make the feature work on more targets than they cared for. On the other hand, if you hide the failures, there is essentially zero chance that architecture maintainers pick up the pieces (I personally implemented the SPARC support only because I had ran into the failures in the testsuite). So doing the inverse filtering sounds quite counterproductive to me and IMO it's up to the architecture maintainers to decide on a case-by-case basis.
On Wed, Mar 31, 2021 at 08:01:29AM +0200, Eric Botcazou wrote: > > It looks like the latter - I've seen no attempt by the original authors to > > make the feature work on more targets than they cared for. > > On the other hand, if you hide the failures, there is essentially zero chance > that architecture maintainers pick up the pieces (I personally implemented the > SPARC support only because I had ran into the failures in the testsuite). So > doing the inverse filtering sounds quite counterproductive to me and IMO it's > up to the architecture maintainers to decide on a case-by-case basis. That is true, but nothing really happened during the 5 months that the tests have been failing on many other architectures (except that powerpc and arm had skipped those tests). There has been a PR open for all those 5 months. We can perhaps revert the skips after branching GCC 11 off, but I have little hope other target maintainers will do what you did, so unsure if it would help. And the changes need people familiar with each of the backends to decide what needs to be done and what is doable. Jakub
> That is true, but nothing really happened during the 5 months that the tests > have been failing on many other architectures (except that powerpc and arm > had skipped those tests). There has been a PR open for all those 5 months. So what? This is not the first example and I don't see anything special with it. You or maintainers can decide to XFAIL particular architectures at will, but hiding the failures by default is IMO not appropriate. > We can perhaps revert the skips after branching GCC 11 off, but I have > little hope other target maintainers will do what you did, so unsure if it > would help. And the changes need people familiar with each of the backends > to decide what needs to be done and what is doable. That's exactly the same situation as for -fstack-usage/-Wstack-usage, where I intentionally made gcc.dg/stack-usage-1.c fail by default so that maintainers could add the missing bits; this worked relatively well.
Yes, basically, I agreed with Eric. One of the major reason to intentionally put these testing cases under c-c++-common is to fail them by default on the platforms that do not support this feature yet. Then the platform maintainer could decide whether to complete this feature on the specific platform or skip them if they don’t want such feature on this platform. Qing > On Mar 31, 2021, at 2:14 AM, Eric Botcazou <botcazou@adacore.com> wrote: > >> That is true, but nothing really happened during the 5 months that the tests >> have been failing on many other architectures (except that powerpc and arm >> had skipped those tests). There has been a PR open for all those 5 months. > > So what? This is not the first example and I don't see anything special with > it. You or maintainers can decide to XFAIL particular architectures at will, > but hiding the failures by default is IMO not appropriate. > >> We can perhaps revert the skips after branching GCC 11 off, but I have >> little hope other target maintainers will do what you did, so unsure if it >> would help. And the changes need people familiar with each of the backends >> to decide what needs to be done and what is doable. > > That's exactly the same situation as for -fstack-usage/-Wstack-usage, where I > intentionally made gcc.dg/stack-usage-1.c fail by default so that maintainers > could add the missing bits; this worked relatively well. > > -- > Eric Botcazou > >
Eric Botcazou <botcazou@adacore.com> writes: >> It looks like the latter - I've seen no attempt by the original authors to >> make the feature work on more targets than they cared for. > > On the other hand, if you hide the failures, there is essentially zero chance > that architecture maintainers pick up the pieces (I personally implemented the > SPARC support only because I had ran into the failures in the testsuite). So > doing the inverse filtering sounds quite counterproductive to me and IMO it's > up to the architecture maintainers to decide on a case-by-case basis. Sorry for the late reply, but +1 FWIW. As Jakub says, this feature doesn't necessarily need work for each target. It doesn't for aarch64, for example, and likely doesn't for several others. But if it does need work, that work requires target-specific knowledge. It isn't reasonable or fair to expect one person to learn enough about each architecture to do the right thing for that architecture. And this isn't new. I've often seen even long-standing GCC developers say things like “I don't know enough about target X to do that. I'll have to leave it to someone who knows about target X”. One of the decisions for maintainers is whether they care about this feature at all. If they don't, they can get GCC to emit a “sorry, not supported”. This decision will be based on maintainer knowledge about how the architecture is used in practice. If maintainers do care, the question then is: which registers should be covered? And what's the best way of zeroing them in the constrained situation that the option is dealing with? (The default does the “obvious” thing in both cases. Intervention is only needed if the default isn't right.) In other words, the ball isn't IMO in the original author's court here. So personally I object to the second patch. I think the first was the right way to go. As it stands we're hiding ICEs through skips rather than xfails, and without a specific list of FIXMEs. Thanks, Richard
--- gcc/testsuite/c-c++-common/zero-scratch-regs-8.c.jj 2020-11-11 01:46:03.392696119 +0100 +++ gcc/testsuite/c-c++-common/zero-scratch-regs-8.c 2021-03-30 12:32:11.099667255 +0200 @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ +/* { dg-skip-if "not implemented" { s390*-*-* } } */ /* { dg-options "-O2 -fzero-call-used-regs=all-arg" } */ #include "zero-scratch-regs-1.c" --- gcc/testsuite/c-c++-common/zero-scratch-regs-9.c.jj 2020-11-11 01:46:03.392696119 +0100 +++ gcc/testsuite/c-c++-common/zero-scratch-regs-9.c 2021-03-30 12:32:26.707493760 +0200 @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ +/* { dg-skip-if "not implemented" { s390*-*-* } } */ /* { dg-options "-O2 -fzero-call-used-regs=all" } */ #include "zero-scratch-regs-1.c" --- gcc/testsuite/c-c++-common/zero-scratch-regs-10.c.jj 2021-03-18 15:32:56.459617723 +0100 +++ gcc/testsuite/c-c++-common/zero-scratch-regs-10.c 2021-03-30 12:31:56.468829910 +0200 @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ +/* { dg-skip-if "not implemented" { s390*-*-* } } */ /* { dg-skip-if "not implemented" { arm*-*-* } } */ /* { dg-options "-O2" } */ --- gcc/testsuite/c-c++-common/zero-scratch-regs-11.c.jj 2020-11-11 01:46:03.392696119 +0100 +++ gcc/testsuite/c-c++-common/zero-scratch-regs-11.c 2021-03-30 12:32:46.012279152 +0200 @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-skip-if "not implemented" { powerpc*-*-* } } */ +/* { dg-skip-if "not implemented" { s390*-*-* } } */ /* { dg-options "-O2 -fzero-call-used-regs=all" } */ #include "zero-scratch-regs-10.c"