add -mfloat128 to __float128-using test missing it

Message ID orsg53b7ba.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • add -mfloat128 to __float128-using test missing it
Related show

Commit Message

Alexandre Oliva March 10, 2021, 11:02 a.m.
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.

This was regstrapped on x86_64-linux-gnu and ppc64-linux-gnu, and tested
with a cross to a ppc64-vxworks7r2.  Ok to install?


for  gcc/testsuite/ChangeLog

	* gcc.target/powerpc/prefix-ds-dq.c: Enable __float128.
---
 gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


-- 
Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

Comments

Alexandre Oliva March 11, 2021, 8:40 a.m. | #1
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, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
Jeff Law via Gcc-patches March 11, 2021, 9:16 a.m. | #2
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" } */
Alexandre Oliva March 11, 2021, 2:56 p.m. | #3
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, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
Jeff Law via Gcc-patches March 11, 2021, 3:07 p.m. | #4
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

Patch

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