public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* add -mfloat128 to __float128-using test missing it
@ 2021-03-10 11:02 Alexandre Oliva
  2021-03-11  8:40 ` Alexandre Oliva
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Oliva @ 2021-03-10 11:02 UTC (permalink / raw)
  To: gcc-patches; +Cc: Rainer Orth, Mike Stump, Segher Boessenkool, David Edelsohn


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(-)

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: add -mfloat128 to __float128-using test missing it
  2021-03-10 11:02 add -mfloat128 to __float128-using test missing it Alexandre Oliva
@ 2021-03-11  8:40 ` Alexandre Oliva
  2021-03-11  9:16   ` Iain Sandoe
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Oliva @ 2021-03-11  8:40 UTC (permalink / raw)
  To: gcc-patches; +Cc: Rainer Orth, Mike Stump, Segher Boessenkool, David Edelsohn

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: add -mfloat128 to __float128-using test missing it
  2021-03-11  8:40 ` Alexandre Oliva
@ 2021-03-11  9:16   ` Iain Sandoe
  2021-03-11 14:56     ` Alexandre Oliva
  2022-04-13 23:47     ` Alexandre Oliva
  0 siblings, 2 replies; 6+ messages in thread
From: Iain Sandoe @ 2021-03-11  9:16 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches, David Edelsohn, Segher Boessenkool

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" } */
 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: add -mfloat128 to __float128-using test missing it
  2021-03-11  9:16   ` Iain Sandoe
@ 2021-03-11 14:56     ` Alexandre Oliva
  2021-03-11 15:07       ` Iain Sandoe
  2022-04-13 23:47     ` Alexandre Oliva
  1 sibling, 1 reply; 6+ messages in thread
From: Alexandre Oliva @ 2021-03-11 14:56 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: gcc-patches, David Edelsohn, Segher Boessenkool

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: add -mfloat128 to __float128-using test missing it
  2021-03-11 14:56     ` Alexandre Oliva
@ 2021-03-11 15:07       ` Iain Sandoe
  0 siblings, 0 replies; 6+ messages in thread
From: Iain Sandoe @ 2021-03-11 15:07 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches, David Edelsohn, Segher Boessenkool

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: add -mfloat128 to __float128-using test missing it
  2021-03-11  9:16   ` Iain Sandoe
  2021-03-11 14:56     ` Alexandre Oliva
@ 2022-04-13 23:47     ` Alexandre Oliva
  1 sibling, 0 replies; 6+ messages in thread
From: Alexandre Oliva @ 2022-04-13 23:47 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: gcc-patches, David Edelsohn, Segher Boessenkool

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

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-04-13 23:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 11:02 add -mfloat128 to __float128-using test missing it Alexandre Oliva
2021-03-11  8:40 ` Alexandre Oliva
2021-03-11  9:16   ` Iain Sandoe
2021-03-11 14:56     ` Alexandre Oliva
2021-03-11 15:07       ` Iain Sandoe
2022-04-13 23:47     ` Alexandre Oliva

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).