Hi Haochen,
on 2024/1/12 14:48, HAO CHEN GUI wrote:
> Hi,
> On P9 "setb" is used to set the result of block compare. So it works
> with m32 and mpowerpc64. On P8, carry bit is used. So it can't work
> with m32 and mpowerpc64. This patch enables block compare expand for
> m32 and mpowerpc64 on P9.
>
> Bootstrapped and tested on x86 and powerpc64-linux BE and LE with no
> regressions. Is this OK for trunk?
OK with two nits below tweaked. Thanks!
BR,
Kewen
>
> Thanks
> Gui Haochen
>
>
> ChangeLog
> rs6000: Enable block compare expand on P9 with m32 and mpowerpc64
>
> gcc/
> * config/rs6000/rs6000-string.cc (expand_block_compare): Enable
> P9 with m32 and mpowerpc64.
>
> gcc/testsuite/
> * gcc.target/powerpc/block-cmp-1.c: Exclude m32 and mpowerpc64.
> * gcc.target/powerpc/block-cmp-4.c: Likewise.
> * gcc.target/powerpc/block-cmp-8.c: New.
>
> patch.diff
> diff --git a/gcc/config/rs6000/rs6000-string.cc
> b/gcc/config/rs6000/rs6000-string.cc
> index 018b87f2501..346708071b5 100644
> --- a/gcc/config/rs6000/rs6000-string.cc
> +++ b/gcc/config/rs6000/rs6000-string.cc
> @@ -1677,11 +1677,12 @@ expand_block_compare (rtx operands[])
> /* TARGET_POPCNTD is already guarded at expand cmpmemsi. */
> gcc_assert (TARGET_POPCNTD);
>
> - /* This case is complicated to handle because the subtract
> - with carry instructions do not generate the 64-bit
> - carry and so we must emit code to calculate it ourselves.
> - We choose not to implement this yet. */
> - if (TARGET_32BIT && TARGET_POWERPC64)
> + /* For P8, this case is complicated to handle because the subtract
> + with carry instructions do not generate the 64-bit carry and so
> + we must emit code to calculate it ourselves. We skip it on P8
> + but setb works well on P9. */
> + if (TARGET_32BIT && TARGET_POWERPC64
Nit: Move "&& TARGET_POWERPC64" as one separated line to make it read better.
> + && !TARGET_P9_MISC)
> return false;
>
> /* Allow this param to shut off all expansion. */
> diff --git a/gcc/testsuite/gcc.target/powerpc/block-cmp-1.c
> b/gcc/testsuite/gcc.target/powerpc/block-cmp-1.c
> index bcf0cb2ab4f..cd076cf1dce 100644
> --- a/gcc/testsuite/gcc.target/powerpc/block-cmp-1.c
> +++ b/gcc/testsuite/gcc.target/powerpc/block-cmp-1.c
> @@ -1,5 +1,6 @@
> /* { dg-do compile } */
> /* { dg-options "-O2 -mdejagnu-cpu=power8 -mno-vsx" } */
> +/* { dg-skip-if "" { has_arch_ppc64 && ilp32 } } */
> /* { dg-final { scan-assembler-not {\mb[l]? memcmp\M} } } */
>
> /* Test that it still can do expand for memcmpsi instead of calling library
> diff --git a/gcc/testsuite/gcc.target/powerpc/block-cmp-4.c
> b/gcc/testsuite/gcc.target/powerpc/block-cmp-4.c
> index c86febae68a..9373b53a3a4 100644
> --- a/gcc/testsuite/gcc.target/powerpc/block-cmp-4.c
> +++ b/gcc/testsuite/gcc.target/powerpc/block-cmp-4.c
> @@ -1,5 +1,6 @@
> /* { dg-do compile { target be } } */
> /* { dg-options "-O2 -mdejagnu-cpu=power7" } */
> +/* { dg-skip-if "" { has_arch_ppc64 && ilp32 } } */
> /* { dg-final { scan-assembler-not {\mb[l]? memcmp\M} } } */
>
> /* Test that it does expand for memcmpsi instead of calling library on
> diff --git a/gcc/testsuite/gcc.target/powerpc/block-cmp-8.c
> b/gcc/testsuite/gcc.target/powerpc/block-cmp-8.c
> new file mode 100644
> index 00000000000..b470f873973
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/block-cmp-8.c
> @@ -0,0 +1,8 @@
> +/* { dg-do run { target ilp32 } } */
> +/* { dg-options "-O2 -m32 -mpowerpc64" } */
Nit: -m32 isn't needed.
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +/* { dg-timeout-factor 2 } */
> +
> +/* Verify memcmp on m32 mpowerpc64 */
> +
> +#include "../../gcc.dg/memcmp-1.c"
BR,
Kewen
