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

[Iain Sandoe <iain@sandoe.co.uk>]

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>