| Message ID | orsg53b7ba.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | add -mfloat128 to __float128-using test missing it | expand |
On Mar 10, 2021, Alexandre Oliva <oliva@adacore.com> wrote:
> * gcc.target/powerpc/prefix-ds-dq.c: Enable __float128.
I've been reminded that this is not enough for the scan-assembler tests
to pass, at least in our configurations. Nearly all of the asm
expectations are unmet. I'm yet to identify the root cause.
Alexandre Oliva <oliva@adacore.com> wrote: > On Mar 10, 2021, Alexandre Oliva <oliva@adacore.com> wrote: > >> * gcc.target/powerpc/prefix-ds-dq.c: Enable __float128. > > I've been reminded that this is not enough for the scan-assembler tests > to pass, at least in our configurations. Nearly all of the asm > expectations are unmet. I'm yet to identify the root cause. I have the following patch that I was intending to apply/post later (since the test makes unconditional use of __float128 which is not available on all platforms). does this solve your issue? Iain gcc/testsuite/ChangeLog: * gcc.target/powerpc/prefix-ds-dq.c: Require float128 support. diff --git a/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c index 554cd0c..7ab7201 100644 --- a/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c +++ b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c @@ -1,5 +1,6 @@ /* { dg-do compile } */ /* { dg-require-effective-target powerpc_prefixed_addr } */ +/* { dg-require-effective-target ppc_float128_sw } */ /* { dg-require-effective-target lp64 } */ /* { dg-options "-O2 -mdejagnu-cpu=power10" } */
On Mar 11, 2021, Iain Sandoe <idsandoe@googlemail.com> wrote: > Alexandre Oliva <oliva@adacore.com> wrote: >> On Mar 10, 2021, Alexandre Oliva <oliva@adacore.com> wrote: >> >>> * gcc.target/powerpc/prefix-ds-dq.c: Enable __float128. >> >> I've been reminded that this is not enough for the scan-assembler tests >> to pass, at least in our configurations. Nearly all of the asm >> expectations are unmet. I'm yet to identify the root cause. > I have the following patch that I was intending to apply/post later (since > the test makes unconditional use of __float128 which is not available on > all platforms). > does this solve your issue? I suppose it would silence the fail by skipping the test. I also *think* it would defeat the purpose of the test, since the requirement test would take effect before -mcpu=power10. My understanding is that this test, being of the "check output asm" kind, rather than "check that it compiles" or "check that it runs without crashing", we'd get better (or at least no-worse) coverage out of any single test run by attempting to perform the test regardless of the powerpc target in use. E.g., I found that, besides -mfloat128, I'd get the expect asm by just cancelling out the -mstrict-align flag that the toolchain I'm testing enables by default. So here's my updated patch, that I'm nearly done retesting. Ok to install? add -mfloat128 to __float128-using test missing it Most (all?) powerpc tests that use the __float128 type either enable it with -mfloat128, or use effective target requirements to check for its presence. prefix-ds-dq.c is failing in some of our configurations because it uses the __float128 type without checking for it, or enabling it explicitly. Since it's a compile test, I'm enabling it explicitly. The flag causes a warning to be printed when float128 may not be entirely supported; I've arranged for the warning to be ignored in this test. In order for the expected asm to be generated, I found the need for -mno-strict-align, on toolchains that enable -mstrict-align by default. for gcc/testsuite/ChangeLog * gcc.target/powerpc/prefix-ds-dq.c: Enable __float128, disable -mstrict-align. --- gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c index 554cd0c1beac0..ddf563521889c 100644 --- a/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c +++ b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ /* { dg-require-effective-target powerpc_prefixed_addr } */ /* { dg-require-effective-target lp64 } */ -/* { dg-options "-O2 -mdejagnu-cpu=power10" } */ +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mfloat128 -mno-strict-align" } */ +/* { dg-prune-output ".-mfloat128. option may not be fully supported" } */ /* Tests whether we generate a prefixed load/store operation for addresses that don't meet DS/DQ offset constraints. 64-bit is needed for testing the use
Alexandre Oliva <oliva@adacore.com> wrote: > On Mar 11, 2021, Iain Sandoe <idsandoe@googlemail.com> wrote: > >> Alexandre Oliva <oliva@adacore.com> wrote: >>> On Mar 10, 2021, Alexandre Oliva <oliva@adacore.com> wrote: >>> >>>> * gcc.target/powerpc/prefix-ds-dq.c: Enable __float128. >>> >>> I've been reminded that this is not enough for the scan-assembler tests >>> to pass, at least in our configurations. Nearly all of the asm >>> expectations are unmet. I'm yet to identify the root cause. > >> I have the following patch that I was intending to apply/post later (since >> the test makes unconditional use of __float128 which is not available on >> all platforms). > >> does this solve your issue? > > I suppose it would silence the fail by skipping the test. I also > *think* it would defeat the purpose of the test, since the requirement > test would take effect before -mcpu=power10. > > My understanding is that this test, being of the "check output asm" > kind, rather than "check that it compiles" or "check that it runs > without crashing", we'd get better (or at least no-worse) coverage out > of any single test run by attempting to perform the test regardless of > the powerpc target in use. I’m all in favour of improved test coverage. > E.g., I found that, besides -mfloat128, I'd get the expect asm by just > cancelling out the -mstrict-align flag that the toolchain I'm testing > enables by default. So here's my updated patch, that I'm nearly done > retesting. Ok to install? > > > add -mfloat128 to __float128-using test missing it > > Most (all?) powerpc tests that use the __float128 type either enable > it with -mfloat128, or use effective target requirements to check for > its presence. > > prefix-ds-dq.c is failing in some of our configurations because it > uses the __float128 type without checking for it, or enabling it > explicitly. > > Since it's a compile test, I'm enabling it explicitly. > > > The flag causes a warning to be printed when float128 may not be > entirely supported; I've arranged for the warning to be ignored in > this test. > > > In order for the expected asm to be generated, I found the need for > -mno-strict-align, on toolchains that enable -mstrict-align by > default. > > > for gcc/testsuite/ChangeLog > > * gcc.target/powerpc/prefix-ds-dq.c: Enable __float128, > disable -mstrict-align. > --- > gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c > b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c > index 554cd0c1beac0..ddf563521889c 100644 > --- a/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c > +++ b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c > @@ -1,7 +1,8 @@ > /* { dg-do compile } */ > /* { dg-require-effective-target powerpc_prefixed_addr } */ > /* { dg-require-effective-target lp64 } */ > -/* { dg-options "-O2 -mdejagnu-cpu=power10" } */ > +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mfloat128 -mno-strict-align" > } */ "-mno-strict-align” is not accepted everywhere, I think. Iain > +/* { dg-prune-output ".-mfloat128. option may not be fully supported" } */ > > /* Tests whether we generate a prefixed load/store operation for > addresses that > don't meet DS/DQ offset constraints. 64-bit is needed for testing the use > > > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
Hello, Iain, Sorry about the late response. On Mar 11, 2021, Iain Sandoe <idsandoe@googlemail.com> wrote: > Alexandre Oliva <oliva@adacore.com> wrote: >> On Mar 10, 2021, Alexandre Oliva <oliva@adacore.com> wrote: >> >>> * gcc.target/powerpc/prefix-ds-dq.c: Enable __float128. >> >> I've been reminded that this is not enough for the scan-assembler tests >> to pass, at least in our configurations. Nearly all of the asm >> expectations are unmet. I'm yet to identify the root cause. > I have the following patch that I was intending to apply/post later (since > the test makes unconditional use of __float128 which is not available on > all platforms). > does this solve your issue? It would disable the compile test, instead of allowing it to do its job. powerpc_prefixed_addr and -mcpu=power10 imply float128 support is available on the hardware, so ppc_float128_sw should be redundant. But it isn't: IIRC it checks for runtime support, without disregarding the warning, and as of a few days ago, it explicitly rejects vxworks targets. So I'd prefer if this didn't go in, and the patch I proposed went in instead. > +/* { dg-require-effective-target ppc_float128_sw } */ So ping? https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566532.html
diff --git a/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c index 554cd0c1beac0..6517eadf44c03 100644 --- a/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c +++ b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ /* { dg-require-effective-target powerpc_prefixed_addr } */ /* { dg-require-effective-target lp64 } */ -/* { dg-options "-O2 -mdejagnu-cpu=power10" } */ +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mfloat128" } */ +/* { dg-prune-output ".-mfloat128. option may not be fully supported" } */ /* Tests whether we generate a prefixed load/store operation for addresses that don't meet DS/DQ offset constraints. 64-bit is needed for testing the use