[PATCH] rs6000: Builtin test changes for int_128bit-runnable.c

Segher Boessenkool segher@kernel.crashing.org
Thu Nov 18 16:22:21 GMT 2021


On Thu, Nov 18, 2021 at 10:09:53AM -0600, Bill Schmidt wrote:
> Hi!  This patch is broken out from the test case patch for the new builtins 
> support.
> 
> The old builtins code performs gimple folding on 128-bit compares.  This
> results in correct but very inefficient code.  (I suspect we may be
> missing some optab entries, misleading gimple into 64-bit emulation.)

It is sub-optimal if Gimple ever does this: it is better done later, at
expand time.

> In
> the new support, I disabled this gimple folding, which results in us
> directly generating the 128-bit comparison instructions.  This patch
> adjusts the scan-assembler-times counts for those instructions.
> 
> I've opened PR103316 to track this.

Thanks.

So when the generic code wisens up this testcase will still pass?  But
you do then need to re-introduce the folding here?

> gcc/testsuite/
> 	* gcc.target/powerpc/int_128bit-runnable.c: Adjust instruction
> 	counts since we do better by not gimple-folding some builtins.

Wrap later please?  80 chars is fine, 79 chars is fine, 10 chars or 70
chars is not :-(

(Not that it matters much *here* of course; it just annoys me

Also, s/ since.*/./ please.  Changelogs say what changed, not why, and
the "why" you say here is only half of the story, pretty misleading for
future archaeologists.

> --- a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
> @@ -11,9 +11,9 @@
>  /* { dg-final { scan-assembler-times {\mvrlq\M} 2 } } */
>  /* { dg-final { scan-assembler-times {\mvrlqnm\M} 2 } } */
>  /* { dg-final { scan-assembler-times {\mvrlqmi\M} 2 } } */
> -/* { dg-final { scan-assembler-times {\mvcmpequq\M} 16 } } */
> -/* { dg-final { scan-assembler-times {\mvcmpgtsq\M} 16 } } */
> -/* { dg-final { scan-assembler-times {\mvcmpgtuq\M} 16 } } */
> +/* { dg-final { scan-assembler-times {\mvcmpequq\M} 24 } } */
> +/* { dg-final { scan-assembler-times {\mvcmpgtsq\M} 26 } } */
> +/* { dg-final { scan-assembler-times {\mvcmpgtuq\M} 26 } } */
>  /* { dg-final { scan-assembler-times {\mvmuloud\M} 1 } } */
>  /* { dg-final { scan-assembler-times {\mvmulesd\M} 1 } } */
>  /* { dg-final { scan-assembler-times {\mvmulosd\M} 1 } } */

If you think it actually generates better code now, and this is expected
code, then okay for trunk.  Thanks!


Segher


More information about the Gcc-patches mailing list