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
