public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*
@ 2023-05-24  5:51 Alexandre Oliva
  2023-05-25  5:44 ` Kewen.Lin
  0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Oliva @ 2023-05-24  5:51 UTC (permalink / raw)
  To: gcc-patches
  Cc: Rainer Orth, Mike Stump, David Edelsohn, Segher Boessenkool, Kewen Lin


Codegen changes caused add instruction count mismatches on
ppc-*-linux-gnu and other 32-bit ppc targets.  At some point the
expected counts were adjusted for lp64, but ilp32 differences
remained, and published test results confirm it.

Bootstrapped on x86_64-linux-gnu.  Also tested on ppc- and x86-vx7r2
with gcc-12.

for  gcc/testsuite/ChangeLog

	* gcc.target/powerpc/fold-vec-extract-char.p7.c: Adjust addi
	counts for ilp32.
	* gcc.target/powerpc/fold-vec-extract-double.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-float.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-float.p8.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-int.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-int.p8.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-short.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-short.p8.c: Likewise.
---
 .../gcc.target/powerpc/fold-vec-extract-char.p7.c  |    3 ++-
 .../powerpc/fold-vec-extract-double.p7.c           |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-float.p7.c |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-float.p8.c |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-int.p7.c   |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-int.p8.c   |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-short.p7.c |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-short.p8.c |    2 +-
 8 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c
index 29a8aa84db282..c6647431d09c9 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c
@@ -11,7 +11,8 @@
 /* one extsb (extend sign-bit) instruction generated for each test against
    unsigned types */
 
-/* { dg-final { scan-assembler-times {\maddi\M} 9 } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 9 { target { lp64 } } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 6 { target { ilp32 } } } } */
 /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
 /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M|\mstxv\M} 6 } } */
 /* -m32 target uses rlwinm in place of rldicl. */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
index 3cae644b90b71..db325efbb07ff 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
@@ -14,7 +14,7 @@
 /* { dg-final { scan-assembler-times {\mli\M} 1 } } */
 /* -m32 target has an 'add' in place of one of the 'addi'. */
 /* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target ilp32 } } } */
 /* -m32 target has a rlwinm in place of a rldic .  */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */
 /* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
index 59a4979457dcb..42ec69475fd07 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
@@ -13,7 +13,7 @@
 /* { dg-final { scan-assembler-times {\mli\M} 1 } } */
 /* -m32 as an add in place of an addi. */
 /* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mstxvd2x\M|\mstvx\M|\mstxv\M} 1 } } */
 /* -m32 uses rlwinm in place of rldic */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
index 4b1d75ee26d0f..68eeeede4b307 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
@@ -26,7 +26,7 @@
 /* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\madd\M} 1 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mlfs\M} 1 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M} 2 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 1 { target ilp32 } } } */
 
 
 #include <altivec.h>
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
index 3729a1646e9c9..e8130693ee953 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
@@ -11,7 +11,7 @@
 
 /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
 /* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 12 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M|\mstxv\M} 6 } } */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 3 } } */
 /* { dg-final { scan-assembler-times {\mlwz\M|\mlwa\M|\mlwzx\M|\mlwax\M} 6 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
index 75eaf25943b70..d1e3b62373f80 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
@@ -30,7 +30,7 @@
 /* { dg-final { scan-assembler-times {\mstxvw4x\M} 6 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\madd\M} 3 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mlwz\M} 6 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M} 9 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
 
 
 
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
index a495d9f3928fa..ec3b78bac5df6 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
@@ -11,7 +11,7 @@
 
 /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
 /* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 12 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 3 } } */
 /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M} 6 } } */
 /* { dg-final { scan-assembler-times "lhz|lha|lhzx|lhax" 6 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
index 0ddecb4e4b55d..00685aca1367b 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
@@ -32,7 +32,7 @@
 /* add and rlwinm instructions only on the variable tests. */
 /* { dg-final { scan-assembler-times {\madd\M} 3 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times "rlwinm" 3 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M} 9 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mlha\M|\mlhz\M} 6 { target ilp32 } } } */
 
 

-- 
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] 8+ messages in thread

* Re: [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*
  2023-05-24  5:51 [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract* Alexandre Oliva
@ 2023-05-25  5:44 ` Kewen.Lin
  2023-05-25 10:05   ` Alexandre Oliva
  0 siblings, 1 reply; 8+ messages in thread
From: Kewen.Lin @ 2023-05-25  5:44 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Rainer Orth, Mike Stump, David Edelsohn, Segher Boessenkool,
	Kewen Lin, gcc-patches

Hi Alexandre,

on 2023/5/24 13:51, Alexandre Oliva wrote:
> 
> Codegen changes caused add instruction count mismatches on
> ppc-*-linux-gnu and other 32-bit ppc targets.  At some point the
> expected counts were adjusted for lp64, but ilp32 differences
> remained, and published test results confirm it.

Thanks for fixing, I tested this on ppc64le and ppc64 {-m64,-m32}
well.

> 
> Bootstrapped on x86_64-linux-gnu.  Also tested on ppc- and x86-vx7r2
> with gcc-12.
> 
> for  gcc/testsuite/ChangeLog

I think this is for PR101169, could you add it as PR marker?

> 
> 	* gcc.target/powerpc/fold-vec-extract-char.p7.c: Adjust addi
> 	counts for ilp32.
> 	* gcc.target/powerpc/fold-vec-extract-double.p7.c: Likewise.
> 	* gcc.target/powerpc/fold-vec-extract-float.p7.c: Likewise.
> 	* gcc.target/powerpc/fold-vec-extract-float.p8.c: Likewise.
> 	* gcc.target/powerpc/fold-vec-extract-int.p7.c: Likewise.
> 	* gcc.target/powerpc/fold-vec-extract-int.p8.c: Likewise.
> 	* gcc.target/powerpc/fold-vec-extract-short.p7.c: Likewise.
> 	* gcc.target/powerpc/fold-vec-extract-short.p8.c: Likewise.
> ---
>  .../gcc.target/powerpc/fold-vec-extract-char.p7.c  |    3 ++-
>  .../powerpc/fold-vec-extract-double.p7.c           |    2 +-
>  .../gcc.target/powerpc/fold-vec-extract-float.p7.c |    2 +-
>  .../gcc.target/powerpc/fold-vec-extract-float.p8.c |    2 +-
>  .../gcc.target/powerpc/fold-vec-extract-int.p7.c   |    2 +-
>  .../gcc.target/powerpc/fold-vec-extract-int.p8.c   |    2 +-
>  .../gcc.target/powerpc/fold-vec-extract-short.p7.c |    2 +-
>  .../gcc.target/powerpc/fold-vec-extract-short.p8.c |    2 +-
>  8 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c
> index 29a8aa84db282..c6647431d09c9 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c
> @@ -11,7 +11,8 @@
>  /* one extsb (extend sign-bit) instruction generated for each test against
>     unsigned types */
> 
> -/* { dg-final { scan-assembler-times {\maddi\M} 9 } } */
> +/* { dg-final { scan-assembler-times {\maddi\M} 9 { target { lp64 } } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M} 6 { target { ilp32 } } } } */
>  /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
>  /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M|\mstxv\M} 6 } } */
>  /* -m32 target uses rlwinm in place of rldicl. */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
> index 3cae644b90b71..db325efbb07ff 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
> @@ -14,7 +14,7 @@
>  /* { dg-final { scan-assembler-times {\mli\M} 1 } } */
>  /* -m32 target has an 'add' in place of one of the 'addi'. */
>  /* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target ilp32 } } } */

So both lp64 and ilp32 have the same count, could we merge it and remove the selectors?

>  /* -m32 target has a rlwinm in place of a rldic .  */
>  /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */
>  /* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
> index 59a4979457dcb..42ec69475fd07 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
> @@ -13,7 +13,7 @@
>  /* { dg-final { scan-assembler-times {\mli\M} 1 } } */
>  /* -m32 as an add in place of an addi. */
>  /* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target ilp32 } } } */

Ditto.

>  /* { dg-final { scan-assembler-times {\mstxvd2x\M|\mstvx\M|\mstxv\M} 1 } } */
>  /* -m32 uses rlwinm in place of rldic */
>  /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
> index 4b1d75ee26d0f..68eeeede4b307 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
> @@ -26,7 +26,7 @@
>  /* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 { target ilp32 } } } */
>  /* { dg-final { scan-assembler-times {\madd\M} 1 { target ilp32 } } } */
>  /* { dg-final { scan-assembler-times {\mlfs\M} 1 { target ilp32 } } } */
> -/* { dg-final { scan-assembler-times {\maddi\M} 2 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M} 1 { target ilp32 } } } */
> 
> 
>  #include <altivec.h>
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
> index 3729a1646e9c9..e8130693ee953 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
> @@ -11,7 +11,7 @@
> 
>  /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
>  /* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target lp64 } } } */
> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 12 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target ilp32 } } } */

Ditto.

>  /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M|\mstxv\M} 6 } } */
>  /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 3 } } */
>  /* { dg-final { scan-assembler-times {\mlwz\M|\mlwa\M|\mlwzx\M|\mlwax\M} 6 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
> index 75eaf25943b70..d1e3b62373f80 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
> @@ -30,7 +30,7 @@
>  /* { dg-final { scan-assembler-times {\mstxvw4x\M} 6 { target ilp32 } } } */
>  /* { dg-final { scan-assembler-times {\madd\M} 3 { target ilp32 } } } */
>  /* { dg-final { scan-assembler-times {\mlwz\M} 6 { target ilp32 } } } */
> -/* { dg-final { scan-assembler-times {\maddi\M} 9 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
> 
> 
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
> index a495d9f3928fa..ec3b78bac5df6 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
> @@ -11,7 +11,7 @@
> 
>  /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
>  /* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target lp64 } } } */
> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 12 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target ilp32 } } } */

Ditto.

BR,
Kewen

>  /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 3 } } */
>  /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M} 6 } } */
>  /* { dg-final { scan-assembler-times "lhz|lha|lhzx|lhax" 6 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
> index 0ddecb4e4b55d..00685aca1367b 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
> @@ -32,7 +32,7 @@
>  /* add and rlwinm instructions only on the variable tests. */
>  /* { dg-final { scan-assembler-times {\madd\M} 3 { target ilp32 } } } */
>  /* { dg-final { scan-assembler-times "rlwinm" 3 { target ilp32 } } } */
> -/* { dg-final { scan-assembler-times {\maddi\M} 9 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
>  /* { dg-final { scan-assembler-times {\mlha\M|\mlhz\M} 6 { target ilp32 } } } */
> 
> 
> 

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

* Re: [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*
  2023-05-25  5:44 ` Kewen.Lin
@ 2023-05-25 10:05   ` Alexandre Oliva
  2023-05-25 11:22     ` Segher Boessenkool
  0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Oliva @ 2023-05-25 10:05 UTC (permalink / raw)
  To: Kewen.Lin
  Cc: Rainer Orth, Mike Stump, David Edelsohn, Segher Boessenkool,
	Kewen Lin, gcc-patches

On May 25, 2023, "Kewen.Lin" <linkw@linux.ibm.com> wrote:

> Thanks for fixing, I tested this on ppc64le and ppc64 {-m64,-m32}
> well.

Thanks!

> I think this is for PR101169, could you add it as PR marker?

Nice, will do!

>> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
>> +/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target ilp32 } } } */

> So both lp64 and ilp32 have the same count, could we merge it and
> remove the selectors?

We could, but...  I thought I wouldn't, since they were different
before, and they're likely to diverge again in the future.  I thought
that combining them might suggest that they ought to be the same, when
we already know that this is not the case.

I'll prepare an alternate patch that combines them.

-- 
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] 8+ messages in thread

* Re: [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*
  2023-05-25 10:05   ` Alexandre Oliva
@ 2023-05-25 11:22     ` Segher Boessenkool
  2023-05-25 13:55       ` Alexandre Oliva
  2023-05-31  9:00       ` [PATCH] " Kewen.Lin
  0 siblings, 2 replies; 8+ messages in thread
From: Segher Boessenkool @ 2023-05-25 11:22 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Kewen.Lin, Rainer Orth, Mike Stump, David Edelsohn, Kewen Lin,
	gcc-patches

Hi!

On Thu, May 25, 2023 at 07:05:55AM -0300, Alexandre Oliva wrote:
> On May 25, 2023, "Kewen.Lin" <linkw@linux.ibm.com> wrote:
> > So both lp64 and ilp32 have the same count, could we merge it and
> > remove the selectors?
> 
> We could, but...  I thought I wouldn't, since they were different
> before, and they're likely to diverge again in the future.  I thought
> that combining them might suggest that they ought to be the same, when
> we already know that this is not the case.
> 
> I'll prepare an alternate patch that combines them.

Fwiw, updating the insn counts blindly like this has very small value on
the one hand, and negative value on the other.  In total, negative
value.

If it is not possible to keep these tests up-to-date easily the test
should be improved.  If tests regressed otoh we should ***not*** paper
over that with patches like this, but investigate what happened instead:
such regressions are *real*.

So which is it here?  I am assuming it is a not-to-well written testcase
without all the necessary noipa attrs, and/or putting more than one
thing to test per function directly.  Insn counts then shift easily if
the compiler decides to factor (CSE etc.) your code differently, but
that is a testcase artifact then, not something we want to adjust counts
for all of the time.

It is feasible to do these insn count things only for trivial tiny
snippets.  Everything bigger will regress all of the time, no one will
look at it properly, and instead people will just do blind "update
counts" patches like this :-/  *Good* insn count tests are quite
valuable, but harder to write.  But maintenance costs noticably bigger
than zero for a testcase are not good, how many testcases do we run in
the testsuite?

So, can we fix the underlying problem here please?

Thanks,


Segher

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

* Re: [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*
  2023-05-25 11:22     ` Segher Boessenkool
@ 2023-05-25 13:55       ` Alexandre Oliva
  2023-05-25 15:33         ` Segher Boessenkool
  2023-05-31  9:00       ` [PATCH] " Kewen.Lin
  1 sibling, 1 reply; 8+ messages in thread
From: Alexandre Oliva @ 2023-05-25 13:55 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Kewen.Lin, Rainer Orth, Mike Stump, David Edelsohn, Kewen Lin,
	gcc-patches

On May 25, 2023, Segher Boessenkool <segher@kernel.crashing.org> wrote:

> Fwiw, updating the insn counts blindly like this

... is a claim that carries a wildly incorrect and insulting underlying
assumption: I've actually identified the corresponding change to the
lp64 tests, compared the effects of the codegen changes, and concluded
the tests needed this changing for ilp32 to keep on testing for the same
thing after code changes brought about by changes that AFAICT had been
well understood when making the lp64 adjustments.

> If it is not possible to keep these tests up-to-date easily

The counts have been stable for a couple of release cycles already.

The change that caused the codegen differences is identified and
understood; the PR confirmed my findings, naming the root cause and the
incomplete testsuite adjustment.

I suspect there may also be ABI-related assumptions implied by the 'add'
counts, but I don't know enough about all the ppc variants to be sure.


Now, if your implied claim is correct that counting 'add/addi'
instructions in these tests is fragile, dropping the checks for those
would probably be best.  But if ppc maintainers seem to have different
opinions as to how to deal with the fallout of that one-time codegen
change, it would be foolish for me to get pulled into the cross fire.

Here's the patch that corrects the long-broken counts, with the
requested adjustments, retested with ppc- and ppc64-vx7r2.  Ok?


[testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*

Codegen changes caused add instruction count mismatches on
ppc-*-linux-gnu and other 32-bit ppc targets.  At some point the
expected counts were adjusted for lp64, but ilp32 differences
remained, and published test results confirm it.


for  gcc/testsuite/ChangeLog

	PR testsuite/101169
	* gcc.target/powerpc/fold-vec-extract-char.p7.c: Adjust addi
	counts for ilp32.
	* gcc.target/powerpc/fold-vec-extract-double.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-float.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-float.p8.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-int.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-int.p8.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-short.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-short.p8.c: Likewise.
---
 .../gcc.target/powerpc/fold-vec-extract-char.p7.c  |    3 ++-
 .../powerpc/fold-vec-extract-double.p7.c           |    3 +--
 .../gcc.target/powerpc/fold-vec-extract-float.p7.c |    3 +--
 .../gcc.target/powerpc/fold-vec-extract-float.p8.c |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-int.p7.c   |    3 +--
 .../gcc.target/powerpc/fold-vec-extract-int.p8.c   |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-short.p7.c |    3 +--
 .../gcc.target/powerpc/fold-vec-extract-short.p8.c |    2 +-
 8 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c
index 29a8aa84db282..c6647431d09c9 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p7.c
@@ -11,7 +11,8 @@
 /* one extsb (extend sign-bit) instruction generated for each test against
    unsigned types */
 
-/* { dg-final { scan-assembler-times {\maddi\M} 9 } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 9 { target { lp64 } } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 6 { target { ilp32 } } } } */
 /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
 /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M|\mstxv\M} 6 } } */
 /* -m32 target uses rlwinm in place of rldicl. */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
index 3cae644b90b71..cbf6cffbeba17 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
@@ -13,8 +13,7 @@
 /* { dg-final { scan-assembler-times {\mxxpermdi\M} 1 } } */
 /* { dg-final { scan-assembler-times {\mli\M} 1 } } */
 /* -m32 target has an 'add' in place of one of the 'addi'. */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 } } */
 /* -m32 target has a rlwinm in place of a rldic .  */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */
 /* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
index 59a4979457dcb..c9abb6c1f352c 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
@@ -12,8 +12,7 @@
 /* { dg-final { scan-assembler-times {\mxscvspdp\M} 1 } } */
 /* { dg-final { scan-assembler-times {\mli\M} 1 } } */
 /* -m32 as an add in place of an addi. */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 } } */
 /* { dg-final { scan-assembler-times {\mstxvd2x\M|\mstvx\M|\mstxv\M} 1 } } */
 /* -m32 uses rlwinm in place of rldic */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
index 4b1d75ee26d0f..68eeeede4b307 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
@@ -26,7 +26,7 @@
 /* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\madd\M} 1 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mlfs\M} 1 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M} 2 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 1 { target ilp32 } } } */
 
 
 #include <altivec.h>
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
index 3729a1646e9c9..418762e3948a5 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
@@ -10,8 +10,7 @@
 // P7 variables:  li, addi, stxvw4x, lwa/lwz
 
 /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 12 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 } } */
 /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M|\mstxv\M} 6 } } */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 3 } } */
 /* { dg-final { scan-assembler-times {\mlwz\M|\mlwa\M|\mlwzx\M|\mlwax\M} 6 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
index 75eaf25943b70..d1e3b62373f80 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
@@ -30,7 +30,7 @@
 /* { dg-final { scan-assembler-times {\mstxvw4x\M} 6 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\madd\M} 3 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mlwz\M} 6 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M} 9 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
 
 
 
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
index a495d9f3928fa..46e943faa6a41 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
@@ -10,8 +10,7 @@
 // P7 (be) constants:            li, addi,              stxvw4x, lha/lhz
 
 /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 12 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 } } */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 3 } } */
 /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M} 6 } } */
 /* { dg-final { scan-assembler-times "lhz|lha|lhzx|lhax" 6 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
index 0ddecb4e4b55d..00685aca1367b 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
@@ -32,7 +32,7 @@
 /* add and rlwinm instructions only on the variable tests. */
 /* { dg-final { scan-assembler-times {\madd\M} 3 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times "rlwinm" 3 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M} 9 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mlha\M|\mlhz\M} 6 { target ilp32 } } } */
 
 


-- 
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] 8+ messages in thread

* Re: [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*
  2023-05-25 13:55       ` Alexandre Oliva
@ 2023-05-25 15:33         ` Segher Boessenkool
  2024-04-22 10:11           ` [PATCH v2] " Alexandre Oliva
  0 siblings, 1 reply; 8+ messages in thread
From: Segher Boessenkool @ 2023-05-25 15:33 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Kewen.Lin, Rainer Orth, Mike Stump, David Edelsohn, Kewen Lin,
	gcc-patches

Hi Alex,

On Thu, May 25, 2023 at 10:55:37AM -0300, Alexandre Oliva wrote:
> On May 25, 2023, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> > Fwiw, updating the insn counts blindly like this
> 
> ... is a claim that carries a wildly incorrect and insulting underlying
> assumption:

Sorry you feel that way.  I'm not even assuming anything :-(

> I've actually identified the corresponding change to the
> lp64 tests, compared the effects of the codegen changes, and concluded
> the tests needed this changing for ilp32 to keep on testing for the same
> thing after code changes brought about by changes that AFAICT had been
> well understood when making the lp64 adjustments.

But you didn't explain any of that (saying it is so is not the same
thing at all as explaining it!)

> > If it is not possible to keep these tests up-to-date easily
> 
> The counts have been stable for a couple of release cycles already.
> 
> The change that caused the codegen differences is identified and
> understood; the PR confirmed my findings, naming the root cause and the
> incomplete testsuite adjustment.

Oh, was this discussed in some PR?  The patch submission should have
carried the conclusions from the discussions there then :-)

> I suspect there may also be ABI-related assumptions implied by the 'add'
> counts, but I don't know enough about all the ppc variants to be sure.

The compiler can and will create all kinds of code for wildly unexpected
reasons.  "add" is dangerous to count already, but it is not as bad as
"addi" :-)

> Now, if your implied claim is correct that counting 'add/addi'
> instructions in these tests is fragile, dropping the checks for those
> would probably be best.

The same is true for almost all instructions.  You can only sanely count
instructions if either you count only unusual insns, or if you test only
*tiny* functions (say five insns, including the blr at the end!)

> But if ppc maintainers seem to have different
> opinions as to how to deal with the fallout of that one-time codegen
> change, it would be foolish for me to get pulled into the cross fire.

There is no crossfire.  I did not dis-approve the patch, just said this
is a high maintenance direction to proceed in.  There has been a lot of
that the last few years, we should improve on that.  It is not about
this patch (only).

> Here's the patch that corrects the long-broken counts, with the
> requested adjustments, retested with ppc- and ppc64-vx7r2.  Ok?

> Codegen changes caused add instruction count mismatches on
> ppc-*-linux-gnu and other 32-bit ppc targets.  At some point the
> expected counts were adjusted for lp64, but ilp32 differences
> remained, and published test results confirm it.

... and this is not something that can be confimed like this.  Just
spend a few minutes more to put *actual numbers* here, with some
indication this is good and correct codegen, so that it is bloody easy
for a reviewer to review and for a maintainer to approve!

>  /* -m32 target has an 'add' in place of one of the 'addi'. */
> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 } } */

Just {\madd} or more conservative {\maddi?\M} then?


Segher

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

* Re: [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*
  2023-05-25 11:22     ` Segher Boessenkool
  2023-05-25 13:55       ` Alexandre Oliva
@ 2023-05-31  9:00       ` Kewen.Lin
  1 sibling, 0 replies; 8+ messages in thread
From: Kewen.Lin @ 2023-05-31  9:00 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Rainer Orth, Mike Stump, David Edelsohn, Kewen Lin, gcc-patches,
	Alexandre Oliva, Vladimir Makarov

Hi Segher,

on 2023/5/25 19:22, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, May 25, 2023 at 07:05:55AM -0300, Alexandre Oliva wrote:
>> On May 25, 2023, "Kewen.Lin" <linkw@linux.ibm.com> wrote:
>>> So both lp64 and ilp32 have the same count, could we merge it and
>>> remove the selectors?
>>
>> We could, but...  I thought I wouldn't, since they were different
>> before, and they're likely to diverge again in the future.  I thought
>> that combining them might suggest that they ought to be the same, when
>> we already know that this is not the case.
>>
>> I'll prepare an alternate patch that combines them.
> 
> Fwiw, updating the insn counts blindly like this has very small value on
> the one hand, and negative value on the other.  In total, negative
> value.
> 
> If it is not possible to keep these tests up-to-date easily the test
> should be improved.  If tests regressed otoh we should ***not*** paper
> over that with patches like this, but investigate what happened instead:
> such regressions are *real*.
> 
> So which is it here?  I am assuming it is a not-to-well written testcase
> without all the necessary noipa attrs, and/or putting more than one
> thing to test per function directly.  Insn counts then shift easily if
> the compiler decides to factor (CSE etc.) your code differently, but
> that is a testcase artifact then, not something we want to adjust counts
> for all of the time.
> 
> It is feasible to do these insn count things only for trivial tiny
> snippets.  Everything bigger will regress all of the time, no one will
> look at it properly, and instead people will just do blind "update
> counts" patches like this :-/  *Good* insn count tests are quite
> valuable, but harder to write.  But maintenance costs noticably bigger
> than zero for a testcase are not good, how many testcases do we run in
> the testsuite?
> 
> So, can we fix the underlying problem here please?

Thanks for all the comments and good question.

I looked into this issue and found the current counts for 32-bit are
mainly for aix (it doesn't need any updates there), and there are some
generated assembly differences between aix 32-bit and 32-bit Linux, and
it seems to be related to if compiler saves the frame pointer or not.

Take a function testbc_var from fold-vec-extract-char.p7.c as example:

#include <altivec.h>

unsigned char
testbc_var (vector bool char vbc2, signed int si)
{
  return vec_extract (vbc2, si);
}

1) on aix 32-bit, with trunk:

.testbc_var:
        li 9,32
        addi 10,1,-64
        stxvw4x 34,10,9
        rlwinm 3,3,0,28,31
        addi 9,3,-64          // these two lines
        add 3,9,1             // can be combined, see below.
        lbz 3,32(3)
        blr

with old gcc (without r11-6615):

.testbc_var:
        addi 10,1,-64
        li 9,32
        stxvw4x 34,10,9
        rlwinm 3,3,0,28,31
        add 3,10,3            // better
        lbz 3,32(3)
        blr

apparently an extra unnecessary addi is created.  The test
case expects one addi to adjust stack for a temp space,
one add to prepare the index for the extracted byte.

2) same thing happens on aix 64-bit and Linux 64-bit:

trunk:

.testbc_var:
        li 9,48
        addi 10,1,-64
        stxvw4x 34,10,9
        rldicl 5,5,0,60
        addi 9,5,-64         // similar to aix 32-bit
        add 5,9,1            // ....
        lbz 3,48(5)
        blr

vs. optimized:

.testbc_var:
        addi 10,1,-64
        li 9,48
        stxvw4x 34,10,9
        rldicl 5,5,0,60
        add 5,10,5           // better
        lbz 3,48(5)
        blr

3) but for Linux 32-bit, they are the same between trunk
and old gcc (without r11-6615):

testbc_var:
        stwu 1,-48(1)
        li 9,16
        rlwinm 3,3,0,28,31
        stxvw4x 34,1,9
        add 3,1,3
        lbz 3,16(3)
        addi 1,1,48
        blr

So the expected count adjusted for aix 32-bit broke Linux
32-bit.

As above, the behavior change (one more addi) on 64-bit and aix
32-bit results in sub-optimal code than before, but we updated
the counts previously, so I changed PR101169's component to
rtl-optimization for further investigation.  From 3), what
Alexandre proposed to fix for Linux 32-bit is actually to restore
the expected count back to before. :)

BR,
Kewen

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

* [PATCH v2] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*
  2023-05-25 15:33         ` Segher Boessenkool
@ 2024-04-22 10:11           ` Alexandre Oliva
  0 siblings, 0 replies; 8+ messages in thread
From: Alexandre Oliva @ 2024-04-22 10:11 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Rainer Orth, Mike Stump, David Edelsohn, Kewen Lin, gcc-patches

Ping?-ish
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/619678.html

It's that time of the year again.  The good news is that this is the
last patch in my ppc*-vxworks7* set ;-)

On May 25, 2023, Segher Boessenkool <segher@kernel.crashing.org> wrote:

> On Thu, May 25, 2023 at 10:55:37AM -0300, Alexandre Oliva wrote:
>> I've actually identified the corresponding change to the
>> lp64 tests, compared the effects of the codegen changes, and concluded
>> the tests needed this changing for ilp32 to keep on testing for the same
>> thing after code changes brought about by changes that AFAICT had been
>> well understood when making the lp64 adjustments.

>> /* -m32 target has an 'add' in place of one of the 'addi'. */
>> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
>> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
>> +/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 } } */

> Just {\madd} or more conservative {\maddi?\M} then?

I've made these changes in the v2 below.

Codegen changes caused add instruction count mismatches on
ppc-*-linux-gnu and other 32-bit ppc targets.  At some point the
expected counts were adjusted for lp64, but ilp32 differences
remained, and published test results confirm it.

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


for  gcc/testsuite/ChangeLog

	PR testsuite/101169
	* gcc.target/powerpc/fold-vec-extract-double.p7.c: Adjust addi
	counts for ilp32.
	* gcc.target/powerpc/fold-vec-extract-float.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-float.p8.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-int.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-int.p8.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-short.p7.c: Likewise.
	* gcc.target/powerpc/fold-vec-extract-short.p8.c: Likewise.
---
 .../powerpc/fold-vec-extract-double.p7.c           |    5 ++---
 .../gcc.target/powerpc/fold-vec-extract-float.p7.c |    5 ++---
 .../gcc.target/powerpc/fold-vec-extract-float.p8.c |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-int.p7.c   |    3 +--
 .../gcc.target/powerpc/fold-vec-extract-int.p8.c   |    2 +-
 .../gcc.target/powerpc/fold-vec-extract-short.p7.c |    3 +--
 .../gcc.target/powerpc/fold-vec-extract-short.p8.c |    2 +-
 7 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
index 3cae644b90b71..e69d9253e2d28 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c
@@ -13,12 +13,11 @@
 /* { dg-final { scan-assembler-times {\mxxpermdi\M} 1 } } */
 /* { dg-final { scan-assembler-times {\mli\M} 1 } } */
 /* -m32 target has an 'add' in place of one of the 'addi'. */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi?\M} 2 } } */
 /* -m32 target has a rlwinm in place of a rldic .  */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */
 /* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 } } */
-/* { dg-final { scan-assembler-times {\mlfdx\M|\mlfd\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mlfdx?\M} 1 } } */
 
 #include <altivec.h>
 
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
index 59a4979457dcb..9ff197a704906 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c
@@ -12,13 +12,12 @@
 /* { dg-final { scan-assembler-times {\mxscvspdp\M} 1 } } */
 /* { dg-final { scan-assembler-times {\mli\M} 1 } } */
 /* -m32 as an add in place of an addi. */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi?\M} 2 } } */
 /* { dg-final { scan-assembler-times {\mstxvd2x\M|\mstvx\M|\mstxv\M} 1 } } */
 /* -m32 uses rlwinm in place of rldic */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */
 /* -m32 has lfs in place of lfsx */
-/* { dg-final { scan-assembler-times {\mlfsx\M|\mlfs\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mlfsx?\M} 1 } } */
 
 #include <altivec.h>
 
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
index ce4e43c1fb462..cd80c5e1b19c6 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c
@@ -26,7 +26,7 @@
 /* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\madd\M} 1 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mlfs\M} 1 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M} 2 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 1 { target ilp32 } } } */
 
 
 #include <altivec.h>
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
index 3729a1646e9c9..cc3c803b49cf6 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p7.c
@@ -10,8 +10,7 @@
 // P7 variables:  li, addi, stxvw4x, lwa/lwz
 
 /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 12 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi?\M} 9 } } */
 /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M|\mstxv\M} 6 } } */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 3 } } */
 /* { dg-final { scan-assembler-times {\mlwz\M|\mlwa\M|\mlwzx\M|\mlwax\M} 6 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
index 152fbdd041bec..67db0306df92d 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c
@@ -30,7 +30,7 @@
 /* { dg-final { scan-assembler-times {\mstxvw4x\M} 6 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\madd\M} 3 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mlwz\M} 6 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M} 9 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
 
 
 
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
index a495d9f3928fa..e16277df847bc 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p7.c
@@ -10,8 +10,7 @@
 // P7 (be) constants:            li, addi,              stxvw4x, lha/lhz
 
 /* { dg-final { scan-assembler-times {\mli\M} 6 } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 9 { target lp64 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 12 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi?\M} 9 } } */
 /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 3 } } */
 /* { dg-final { scan-assembler-times {\mstxvw4x\M|\mstvx\M} 6 } } */
 /* { dg-final { scan-assembler-times "lhz|lha|lhzx|lhax" 6 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
index 9eabc5068d495..45a07d10ea96a 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
@@ -32,7 +32,7 @@
 /* add and rlwinm instructions only on the variable tests. */
 /* { dg-final { scan-assembler-times {\madd\M} 3 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times "rlwinm" 3 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times {\maddi\M} 9 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times {\mlha\M|\mlhz\M} 6 { target ilp32 } } } */
 
 


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

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

end of thread, other threads:[~2024-04-22 10:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-24  5:51 [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract* Alexandre Oliva
2023-05-25  5:44 ` Kewen.Lin
2023-05-25 10:05   ` Alexandre Oliva
2023-05-25 11:22     ` Segher Boessenkool
2023-05-25 13:55       ` Alexandre Oliva
2023-05-25 15:33         ` Segher Boessenkool
2024-04-22 10:11           ` [PATCH v2] " Alexandre Oliva
2023-05-31  9:00       ` [PATCH] " Kewen.Lin

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