enable sqrt insns for cdce3.c

enable sqrt insns for cdce3.c

[Alexandre Oliva]

The test expects shrink-wrapping of the fsqrt call, but that will only
occur when there is a usable sqrt insn.

Arrange for dejagnu to add the options that enable the sqrt insn, if
one is available, and to skip the test otherwise.


H-P, this *should* obviate the mmix-specific dg-skip-if.  Would it be
easy for you to confirm that this is the case and, if so, drop it?

This was regstrapped on x86_64-linux-gnu, tested with a cross to a
ppc64-vxworks7r2 configured for a cpu that doesn't have fsqrt enabled,
and I'm now also regstrapping on ppc64-linux-gnu just to be sure.
Ok to install?


for  gcc/testsuite/ChangeLog

	* gcc.dg/cdce3.c: Add sqrt insn options.
---
 gcc/testsuite/gcc.dg/cdce3.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/cdce3.c b/gcc/testsuite/gcc.dg/cdce3.c
index 601ddf055fd71..f9b3633e66a57 100644
--- a/gcc/testsuite/gcc.dg/cdce3.c
+++ b/gcc/testsuite/gcc.dg/cdce3.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target hard_float } */
 /* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -fdump-tree-optimized" } */
-/* { dg-final { scan-tree-dump "cdce3.c:11: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
+/* { dg-add-options sqrt_insn } */
+/* { dg-final { scan-tree-dump "cdce3.c:12: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
 /* { dg-final { scan-tree-dump "sqrtf \\(\[^\n\r]*\\); \\\[tail call\\\]" "optimized" } } */
 /* { dg-skip-if "doesn't have a sqrtf insn" { mmix-*-* } } */
 


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

Re: enable sqrt insns for cdce3.c

[Hans-Peter Nilsson]

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

On Wed, 10 Mar 2021, Alexandre Oliva wrote:

>
> The test expects shrink-wrapping of the fsqrt call, but that will only
> occur when there is a usable sqrt insn.
>
> Arrange for dejagnu to add the options that enable the sqrt insn, if
> one is available, and to skip the test otherwise.
>
>
> H-P, this *should* obviate the mmix-specific dg-skip-if.

Unfortunately it doesn't.

>  Would it be
> easy for you to confirm that this is the case and, if so, drop it?

About as easy as for anyone (this is a compile-test), but no
problem.  Unfortunately I get, with your patch applied and the
dg-skip-if removed:

FAIL: gcc.dg/cdce3.c scan-tree-dump cdce "cdce3.c:12: [^\n\r]*
function call is shrink-wrapped into error conditions."

The dump files and assembly file show no obvious clues to me as
to what is supposed to happen; attached.

brgds, H-P

[-- Attachment #2: Type: text/plain, Size: 209 bytes --]


;; Function foo (foo, funcdef_no=0, decl_uid=1421, cgraph_uid=1, symbol_order=0)

float foo (float x)
{
  float _4;

  <bb 2> [local count: 1073741824]:
  _4 = sqrtf (x_2(D));
  return _4;

}



[-- Attachment #3: Type: text/plain, Size: 221 bytes --]


;; Function foo (foo, funcdef_no=0, decl_uid=1421, cgraph_uid=1, symbol_order=0)

float foo (float x)
{
  float _4;

  <bb 2> [local count: 1073741824]:
  _4 = sqrtf (x_2(D)); [tail call]
  return _4;

}



[-- Attachment #4: Type: text/plain, Size: 373 bytes --]

# 1 "cdce3.c"
! mmixal:= 8H LOC Data_Section
	.text ! mmixal:= 9H LOC 8B
	.p2align 2
	LOC @+(4-@)&3
	.global foo
foo	IS @
	GET $1,rJ
	SET $3,$0
	PUSHJ $2,sqrtf
	PUT rJ,$1
	SET $0,$2
	POP 1,0

	.ident	"GCC: (GNU) 11.0.1 20210309 (experimental) [master revision 0455cd76b687:0ed66571b1d9:63d74fed4566f1de583c368ecb9e2fc423fb1c87]"
	.data ! mmixal:= 8H LOC 9B

Re: enable sqrt insns for cdce3.c

[Alexandre Oliva]

On Mar 10, 2021, Hans-Peter Nilsson <hp@bitrange.com> wrote:

> On Wed, 10 Mar 2021, Alexandre Oliva wrote:
>> 
>> The test expects shrink-wrapping of the fsqrt call, but that will only
>> occur when there is a usable sqrt insn.
>> 
>> Arrange for dejagnu to add the options that enable the sqrt insn, if
>> one is available, and to skip the test otherwise.
>> 
>> 
>> H-P, this *should* obviate the mmix-specific dg-skip-if.

> Unfortunately it doesn't.

Uhh :-(

>> Would it be
>> easy for you to confirm that this is the case and, if so, drop it?

> About as easy as for anyone (this is a compile-test),

I figured you'd have a recent toolchain around ;-)  Thanks!

> FAIL: gcc.dg/cdce3.c scan-tree-dump cdce "cdce3.c:12: [^\n\r]*
> function call is shrink-wrapped into error conditions."

How surprising!  My understanding was that dg-add-options <feature>
implicitly implies dg-require-effective-target <feature>.  That was how
I'd read et-dg-runtest.

Now I see the _runtime after the check_effective_target_${target} there,
and my understanding is updated, so some pending changes may need
revisiting.  Oh my...


> The dump files and assembly file show no obvious clues to me as
> to what is supposed to happen; attached.

The test just should keep on not running on mmix.  Without an insn for
sqrt, there's no point in shrink-wrapping, and the code that performs
the optimization is smart enough to realize that, so it just leaves the
code alone.


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

Re: enable sqrt insns for cdce3.c

[Alexandre Oliva]

[Revamped version of this patch, combined with others, to follow]

On Mar 10, 2021, Hans-Peter Nilsson <hp@bitrange.com> wrote:

> On Wed, 10 Mar 2021, Alexandre Oliva wrote:
>> 
>> The test expects shrink-wrapping of the fsqrt call, but that will only
>> occur when there is a usable sqrt insn.
>> 
>> Arrange for dejagnu to add the options that enable the sqrt insn, if
>> one is available, and to skip the test otherwise.
>> 
>> 
>> H-P, this *should* obviate the mmix-specific dg-skip-if.

> Unfortunately it doesn't.

>> Would it be
>> easy for you to confirm that this is the case and, if so, drop it?

> About as easy as for anyone (this is a compile-test), but no
> problem.  Unfortunately I get, with your patch applied and the
> dg-skip-if removed:

> FAIL: gcc.dg/cdce3.c scan-tree-dump cdce "cdce3.c:12: [^\n\r]*
> function call is shrink-wrapped into error conditions."

Is mmix a sqrt_insn effective target?  proc
check_effective_target_sqrt_insn in
gcc/testsuite/lib/target-supports.exp suggests it shouldn't pass, so I'm
surprised it would still try to run the test despite the added
/* { dg-require-effective-target sqrt_insn } */ directive.


> The dump files and assembly file show no obvious clues to me as
> to what is supposed to happen; attached.

cdce3 is supposed to shrink-wrap the sqrtf(x) call into something like
(x >= 0 ? .SQRT(x) : sqrtf(x)), where .SQRT stands for a square root
instruction.

Since we don't know why it still runs for you, I'm keeping the mmix
explicit skip in the new version of the patch.

Thanks,

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: enable sqrt insns for cdce3.c

[Hans-Peter Nilsson]

On Mon, 22 Apr 2024, Alexandre Oliva wrote:
> [Revamped version of this patch, combined with others, to follow]
> 
> On Mar 10, 2021, Hans-Peter Nilsson <hp@bitrange.com> wrote:

Time flies...

> > On Wed, 10 Mar 2021, Alexandre Oliva wrote:

> Is mmix a sqrt_insn effective target?  proc
> check_effective_target_sqrt_insn in
> gcc/testsuite/lib/target-supports.exp suggests it shouldn't pass, so I'm
> surprised it would still try to run the test despite the added
> /* { dg-require-effective-target sqrt_insn } */ directive.

The effective-target sqrt_insn predicate says "supports hardware 
square root instructions" and doesn't make a difference between 
sqrtdf2 (double) and sqrtsf3 (float).  I'm extrapolating that 
the "divine meaning" of the comment is that such an instruction 
must be present for all supported floating-point modes for the 
predicate to yield true (when the predicate is correctly 
implemented).

(We could also fix the predicate description to actually say 
"for all floating-point modes" and/or split the predicate into 
mode-specific variants, etc. ;-)

MMIX has sqrtdf2 but not sqrtsf2, and the latter is what's used 
in cdce3.c.

> cdce3 is supposed to shrink-wrap the sqrtf(x) call into something like
> (x >= 0 ? .SQRT(x) : sqrtf(x)), where .SQRT stands for a square root
> instruction.

...for 32-bit single floats.

> Since we don't know why it still runs for you, I'm keeping the mmix
> explicit skip in the new version of the patch.

Thanks, that does seem like TRT.

brgds, H-P

Re: enable sqrt insns for cdce3.c

[Alexandre Oliva]

On Apr 23, 2024, Hans-Peter Nilsson <hp@bitrange.com> wrote:

> (We could also fix the predicate description to actually say 
> "for all floating-point modes" and/or split the predicate into 
> mode-specific variants, etc. ;-)

Yeah, I suppose that could make sense.

> MMIX has sqrtdf2 but not sqrtsf2, and the latter is what's used 
> in cdce3.c.

I see, thanks for the info.

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: enable sqrt insns for cdce3.c

[Jeff Law]

On 3/9/2021 11:30 PM, Alexandre Oliva wrote:
> The test expects shrink-wrapping of the fsqrt call, but that will only
> occur when there is a usable sqrt insn.
>
> Arrange for dejagnu to add the options that enable the sqrt insn, if
> one is available, and to skip the test otherwise.
>
>
> H-P, this *should* obviate the mmix-specific dg-skip-if.  Would it be
> easy for you to confirm that this is the case and, if so, drop it?
>
> This was regstrapped on x86_64-linux-gnu, tested with a cross to a
> ppc64-vxworks7r2 configured for a cpu that doesn't have fsqrt enabled,
> and I'm now also regstrapping on ppc64-linux-gnu just to be sure.
> Ok to install?
>
>
> for  gcc/testsuite/ChangeLog
>
> 	* gcc.dg/cdce3.c: Add sqrt insn options.

OK

jeff

[PATCH v2] [testsuite] require sqrt_insn effective target where needed

[Alexandre Oliva]

This patch takes feedback received for 3 earlier patches, and adopts a
simpler approach to skip the still-failing tests, that I believe to be
in line with ppc maintainers' expressed preferences.
https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565939.html
https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566617.html
https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566521.html
Ping?-ish :-)


Some tests fail on ppc and ppc64 when testing a compiler [with options
for] for a CPU [emulator] that doesn't support the sqrt insn.

The gcc.dg/cdce3.c is one in which the expected shrink-wrap
optimization only takes place when the target CPU supports a sqrt
insn.

The gcc.target/powerpc/pr46728-1[0-4].c tests use -mpowerpc-gpopt and
call sqrt(), which involves the sqrt insn that the target CPU under
test may not support.

Require a sqrt_insn effective target for all the affected tests.

Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu.  Also testing
with gcc-13 on ppc64-vx7r2 and ppc-vx7r2.  Ok to install?


for  gcc/testsuite/ChangeLog

	* gcc.dg/cdce3.c: Require sqrt_insn effective target.
	* gcc.target/powerpc/pr46728-10.c: Likewise.
	* gcc.target/powerpc/pr46728-11.c: Likewise.
	* gcc.target/powerpc/pr46728-13.c: Likewise.
	* gcc.target/powerpc/pr46728-14.c: Likewise.
---
 gcc/testsuite/gcc.dg/cdce3.c                  |    3 ++-
 gcc/testsuite/gcc.target/powerpc/pr46728-10.c |    1 +
 gcc/testsuite/gcc.target/powerpc/pr46728-11.c |    1 +
 gcc/testsuite/gcc.target/powerpc/pr46728-13.c |    1 +
 gcc/testsuite/gcc.target/powerpc/pr46728-14.c |    1 +
 5 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/cdce3.c b/gcc/testsuite/gcc.dg/cdce3.c
index 601ddf055fd71..f759a95972e8b 100644
--- a/gcc/testsuite/gcc.dg/cdce3.c
+++ b/gcc/testsuite/gcc.dg/cdce3.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target hard_float } */
+/* { dg-require-effective-target sqrt_insn } */
 /* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -fdump-tree-optimized" } */
-/* { dg-final { scan-tree-dump "cdce3.c:11: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
+/* { dg-final { scan-tree-dump "cdce3.c:12: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
 /* { dg-final { scan-tree-dump "sqrtf \\(\[^\n\r]*\\); \\\[tail call\\\]" "optimized" } } */
 /* { dg-skip-if "doesn't have a sqrtf insn" { mmix-*-* } } */
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
index 3be4728d333a4..7e9bb638106c2 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
@@ -1,6 +1,7 @@
 /* { dg-do run } */
 /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
 /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
+/* { dg-require-effective-target sqrt_insn } */
 
 #include <math.h>
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
index 43b6728a4b812..5bfa25925675a 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
@@ -1,6 +1,7 @@
 /* { dg-do run } */
 /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
 /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
+/* { dg-require-effective-target sqrt_insn } */
 
 #include <math.h>
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
index b9fd63973b728..b66d0209a5e54 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
@@ -1,6 +1,7 @@
 /* { dg-do run } */
 /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
 /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
+/* { dg-require-effective-target sqrt_insn } */
 
 #include <math.h>
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
index 5affff13bdb6c..71a1a70c4e7a2 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
@@ -1,6 +1,7 @@
 /* { dg-do run } */
 /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
 /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
+/* { dg-require-effective-target sqrt_insn } */
 
 #include <math.h>
 

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: [PATCH v2] [testsuite] require sqrt_insn effective target where needed

[Kewen.Lin]

Hi,

on 2024/4/22 17:56, Alexandre Oliva wrote:
> This patch takes feedback received for 3 earlier patches, and adopts a
> simpler approach to skip the still-failing tests, that I believe to be
> in line with ppc maintainers' expressed preferences.
> https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565939.html
> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566617.html
> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566521.html
> Ping?-ish :-)
> 
> 
> Some tests fail on ppc and ppc64 when testing a compiler [with options
> for] for a CPU [emulator] that doesn't support the sqrt insn.
> 
> The gcc.dg/cdce3.c is one in which the expected shrink-wrap
> optimization only takes place when the target CPU supports a sqrt
> insn.
> 
> The gcc.target/powerpc/pr46728-1[0-4].c tests use -mpowerpc-gpopt and
> call sqrt(), which involves the sqrt insn that the target CPU under
> test may not support.
> 
> Require a sqrt_insn effective target for all the affected tests.
> 
> Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu.  Also testing
> with gcc-13 on ppc64-vx7r2 and ppc-vx7r2.  Ok to install?
> 
> 
> for  gcc/testsuite/ChangeLog
> 
> 	* gcc.dg/cdce3.c: Require sqrt_insn effective target.
> 	* gcc.target/powerpc/pr46728-10.c: Likewise.
> 	* gcc.target/powerpc/pr46728-11.c: Likewise.
> 	* gcc.target/powerpc/pr46728-13.c: Likewise.
> 	* gcc.target/powerpc/pr46728-14.c: Likewise.
> ---
>  gcc/testsuite/gcc.dg/cdce3.c                  |    3 ++-
>  gcc/testsuite/gcc.target/powerpc/pr46728-10.c |    1 +
>  gcc/testsuite/gcc.target/powerpc/pr46728-11.c |    1 +
>  gcc/testsuite/gcc.target/powerpc/pr46728-13.c |    1 +
>  gcc/testsuite/gcc.target/powerpc/pr46728-14.c |    1 +
>  5 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/cdce3.c b/gcc/testsuite/gcc.dg/cdce3.c
> index 601ddf055fd71..f759a95972e8b 100644
> --- a/gcc/testsuite/gcc.dg/cdce3.c
> +++ b/gcc/testsuite/gcc.dg/cdce3.c
> @@ -1,7 +1,8 @@
>  /* { dg-do compile } */
>  /* { dg-require-effective-target hard_float } */
> +/* { dg-require-effective-target sqrt_insn } */
>  /* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -fdump-tree-optimized" } */
> -/* { dg-final { scan-tree-dump "cdce3.c:11: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
> +/* { dg-final { scan-tree-dump "cdce3.c:12: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
>  /* { dg-final { scan-tree-dump "sqrtf \\(\[^\n\r]*\\); \\\[tail call\\\]" "optimized" } } */
>  /* { dg-skip-if "doesn't have a sqrtf insn" { mmix-*-* } } */
> 

This change needs an approval from global maintainer as it touches a generic test case?

> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
> index 3be4728d333a4..7e9bb638106c2 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
> @@ -1,6 +1,7 @@
>  /* { dg-do run } */
>  /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>  /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
> +/* { dg-require-effective-target sqrt_insn } */

This change looks sensible to me.

Nit: With the proposed change, I'd expect that we can remove the line for powerpc*-*-darwin*.

CC Iain to confirm.

BR,
Kewen

> 
>  #include <math.h>
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
> index 43b6728a4b812..5bfa25925675a 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
> @@ -1,6 +1,7 @@
>  /* { dg-do run } */
>  /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>  /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
> +/* { dg-require-effective-target sqrt_insn } */
> 
>  #include <math.h>
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
> index b9fd63973b728..b66d0209a5e54 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
> @@ -1,6 +1,7 @@
>  /* { dg-do run } */
>  /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>  /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
> +/* { dg-require-effective-target sqrt_insn } */
> 
>  #include <math.h>
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
> index 5affff13bdb6c..71a1a70c4e7a2 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
> @@ -1,6 +1,7 @@
>  /* { dg-do run } */
>  /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>  /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
> +/* { dg-require-effective-target sqrt_insn } */
> 
>  #include <math.h>
> 
> 

Re: [PATCH v2] [testsuite] require sqrt_insn effective target where needed

[Iain Sandoe]

Hi Folks,

> On 23 Apr 2024, at 09:59, Kewen.Lin <linkw@linux.ibm.com> wrote:
> 
> Hi,
> 
> on 2024/4/22 17:56, Alexandre Oliva wrote:
>> This patch takes feedback received for 3 earlier patches, and adopts a
>> simpler approach to skip the still-failing tests, that I believe to be
>> in line with ppc maintainers' expressed preferences.
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565939.html
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566617.html
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566521.html
>> Ping?-ish :-)
>> 
>> 
>> Some tests fail on ppc and ppc64 when testing a compiler [with options
>> for] for a CPU [emulator] that doesn't support the sqrt insn.
>> 
>> The gcc.dg/cdce3.c is one in which the expected shrink-wrap
>> optimization only takes place when the target CPU supports a sqrt
>> insn.
>> 
>> The gcc.target/powerpc/pr46728-1[0-4].c tests use -mpowerpc-gpopt and
>> call sqrt(), which involves the sqrt insn that the target CPU under
>> test may not support.
>> 
>> Require a sqrt_insn effective target for all the affected tests.
>> 
>> Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu.  Also testing
>> with gcc-13 on ppc64-vx7r2 and ppc-vx7r2.  Ok to install?
>> 
>> 
>> for  gcc/testsuite/ChangeLog
>> 
>> 	* gcc.dg/cdce3.c: Require sqrt_insn effective target.
>> 	* gcc.target/powerpc/pr46728-10.c: Likewise.
>> 	* gcc.target/powerpc/pr46728-11.c: Likewise.
>> 	* gcc.target/powerpc/pr46728-13.c: Likewise.
>> 	* gcc.target/powerpc/pr46728-14.c: Likewise.
>> ---
>> gcc/testsuite/gcc.dg/cdce3.c                  |    3 ++-
>> gcc/testsuite/gcc.target/powerpc/pr46728-10.c |    1 +
>> gcc/testsuite/gcc.target/powerpc/pr46728-11.c |    1 +
>> gcc/testsuite/gcc.target/powerpc/pr46728-13.c |    1 +
>> gcc/testsuite/gcc.target/powerpc/pr46728-14.c |    1 +
>> 5 files changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/gcc/testsuite/gcc.dg/cdce3.c b/gcc/testsuite/gcc.dg/cdce3.c
>> index 601ddf055fd71..f759a95972e8b 100644
>> --- a/gcc/testsuite/gcc.dg/cdce3.c
>> +++ b/gcc/testsuite/gcc.dg/cdce3.c
>> @@ -1,7 +1,8 @@
>> /* { dg-do compile } */
>> /* { dg-require-effective-target hard_float } */
>> +/* { dg-require-effective-target sqrt_insn } */
>> /* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -fdump-tree-optimized" } */
>> -/* { dg-final { scan-tree-dump "cdce3.c:11: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
>> +/* { dg-final { scan-tree-dump "cdce3.c:12: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
>> /* { dg-final { scan-tree-dump "sqrtf \\(\[^\n\r]*\\); \\\[tail call\\\]" "optimized" } } */
>> /* { dg-skip-if "doesn't have a sqrtf insn" { mmix-*-* } } */
>> 
> 
> This change needs an approval from global maintainer as it touches a generic test case?
> 
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
>> index 3be4728d333a4..7e9bb638106c2 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
>> @@ -1,6 +1,7 @@
>> /* { dg-do run } */
>> /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>> /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
>> +/* { dg-require-effective-target sqrt_insn } */
> 
> This change looks sensible to me.
> 
> Nit: With the proposed change, I'd expect that we can remove the line for powerpc*-*-darwin*.
> 
> CC Iain to confirm.

Indeed, the check for sqrt_insn fails and so the test is unsupported without needing the separate
powerpc*-*-darwin* line,

thanks,
Iain

> 
> BR,
> Kewen
> 
>> 
>> #include <math.h>
>> 
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
>> index 43b6728a4b812..5bfa25925675a 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
>> @@ -1,6 +1,7 @@
>> /* { dg-do run } */
>> /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>> /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
>> +/* { dg-require-effective-target sqrt_insn } */
>> 
>> #include <math.h>
>> 
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
>> index b9fd63973b728..b66d0209a5e54 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
>> @@ -1,6 +1,7 @@
>> /* { dg-do run } */
>> /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>> /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
>> +/* { dg-require-effective-target sqrt_insn } */
>> 
>> #include <math.h>
>> 
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
>> index 5affff13bdb6c..71a1a70c4e7a2 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
>> @@ -1,6 +1,7 @@
>> /* { dg-do run } */
>> /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>> /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
>> +/* { dg-require-effective-target sqrt_insn } */
>> 
>> #include <math.h>

Re: [PATCH v2] [testsuite] require sqrt_insn effective target where needed

[Alexandre Oliva]

On Apr 23, 2024, Iain Sandoe <iain@sandoe.co.uk> wrote:

>>> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
>>> @@ -1,6 +1,7 @@
>>> /* { dg-do run } */
>>> /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>>> /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
>>> +/* { dg-require-effective-target sqrt_insn } */
>> 
>> This change looks sensible to me.
>> 
>> Nit: With the proposed change, I'd expect that we can remove the line for powerpc*-*-darwin*.
>> 
>> CC Iain to confirm.

> Indeed, the check for sqrt_insn fails and so the test is unsupported without needing the separate
> powerpc*-*-darwin* line,

Thanks, here's the adjusted version I'm just about to push.


[testsuite] require sqrt_insn effective target where needed

Some tests fail on ppc and ppc64 when testing a compiler [with options
for] for a CPU [emulator] that doesn't support the sqrt insn.

The gcc.dg/cdce3.c is one in which the expected shrink-wrap
optimization only takes place when the target CPU supports a sqrt
insn.

The gcc.target/powerpc/pr46728-1[0-4].c tests use -mpowerpc-gpopt and
call sqrt(), which involves the sqrt insn that the target CPU under
test may not support.

Require a sqrt_insn effective target for all the affected tests.


for  gcc/testsuite/ChangeLog

	* gcc.dg/cdce3.c: Require sqrt_insn effective target.
	* gcc.target/powerpc/pr46728-10.c: Likewise.  Drop darwin
	explicit skipping.
	* gcc.target/powerpc/pr46728-11.c: Likewise.  Likewise.
	* gcc.target/powerpc/pr46728-13.c: Likewise.  Likewise.
	* gcc.target/powerpc/pr46728-14.c: Likewise.  Likewise.
---
 gcc/testsuite/gcc.dg/cdce3.c                  |    3 ++-
 gcc/testsuite/gcc.target/powerpc/pr46728-10.c |    2 +-
 gcc/testsuite/gcc.target/powerpc/pr46728-11.c |    2 +-
 gcc/testsuite/gcc.target/powerpc/pr46728-13.c |    2 +-
 gcc/testsuite/gcc.target/powerpc/pr46728-14.c |    2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/cdce3.c b/gcc/testsuite/gcc.dg/cdce3.c
index 601ddf055fd71..f759a95972e8b 100644
--- a/gcc/testsuite/gcc.dg/cdce3.c
+++ b/gcc/testsuite/gcc.dg/cdce3.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target hard_float } */
+/* { dg-require-effective-target sqrt_insn } */
 /* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -fdump-tree-optimized" } */
-/* { dg-final { scan-tree-dump "cdce3.c:11: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
+/* { dg-final { scan-tree-dump "cdce3.c:12: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */
 /* { dg-final { scan-tree-dump "sqrtf \\(\[^\n\r]*\\); \\\[tail call\\\]" "optimized" } } */
 /* { dg-skip-if "doesn't have a sqrtf insn" { mmix-*-* } } */
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
index 3be4728d333a4..c04a3101c113f 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
@@ -1,6 +1,6 @@
 /* { dg-do run } */
-/* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
 /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
+/* { dg-require-effective-target sqrt_insn } */
 
 #include <math.h>
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
index 43b6728a4b812..d0e3d60212194 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
@@ -1,6 +1,6 @@
 /* { dg-do run } */
-/* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
 /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
+/* { dg-require-effective-target sqrt_insn } */
 
 #include <math.h>
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
index b9fd63973b728..2b9df737a9b0d 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
@@ -1,6 +1,6 @@
 /* { dg-do run } */
-/* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
 /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
+/* { dg-require-effective-target sqrt_insn } */
 
 #include <math.h>
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
index 5affff13bdb6c..e6836f515e4f8 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
@@ -1,6 +1,6 @@
 /* { dg-do run } */
-/* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
 /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */
+/* { dg-require-effective-target sqrt_insn } */
 
 #include <math.h>
 


-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: [PATCH v2] [testsuite] require sqrt_insn effective target where needed

[Mike Stump]

On Apr 22, 2024, at 2:56 AM, Alexandre Oliva <oliva@adacore.com> wrote:
> 
> This patch takes feedback received for 3 earlier patches, and adopts a
> simpler approach to skip the still-failing tests, that I believe to be
> in line with ppc maintainers' expressed preferences.
> https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565939.html
> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566617.html
> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566521.html
> Ping?-ish :-)
> 
> 
> Some tests fail on ppc and ppc64 when testing a compiler [with options
> for] for a CPU [emulator] that doesn't support the sqrt insn.
> 
> The gcc.dg/cdce3.c is one in which the expected shrink-wrap
> optimization only takes place when the target CPU supports a sqrt
> insn.
> 
> The gcc.target/powerpc/pr46728-1[0-4].c tests use -mpowerpc-gpopt and
> call sqrt(), which involves the sqrt insn that the target CPU under
> test may not support.
> 
> Require a sqrt_insn effective target for all the affected tests.
> 
> Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu.  Also testing
> with gcc-13 on ppc64-vx7r2 and ppc-vx7r2.  Ok to install?

Ok.