Re: [PATCH v4 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Sat, Apr 22, 2023 at 02:36:20PM +0530, Ajit Agarwal wrote:
>         * ree.cc (combline_reaching_defs): Add zero_extend
>         using defined abi interfaces.

Typo.  Also, please don't wrap lines early.  Also, you are missing some
changes in this file in the changelog.

>         (add_removable_extension): use of defined abi interfaces
>         for no reaching defs.

Capital U.

>         (abi_extension_candidate_return_reg_p): New defined ABI function.

What does that even mean?  An "ABI function"?

> --- a/gcc/ree.cc
> +++ b/gcc/ree.cc
> @@ -473,7 +473,8 @@ get_defs (rtx_insn *insn, rtx reg, vec<rtx_insn *> *dest)
>  	break;
>      }
>  
> -  gcc_assert (use != NULL);
> +  if (use == NULL)
> +    return NULL;

If it is suddenly allowed to have nil here, some comment somewhere needs
to change as well.

> new file mode 100644
> index 00000000000..1d443af066a
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */

This is required of all file in g++.target/powerpc/ already:
# Exit immediately if this isn't a PowerPC target.
if {![istarget powerpc*-*-*] } then {
  return
}

so please don't repeat that here.

> +/* { dg-require-effective-target lp64 } */

Is that required?!

> +/* { dg-require-effective-target powerpc_p9vector_ok } */
> +/* { dg-options "-mcpu=power9 -O2 -free" } */

Why does this need p9?  We should test this on older systems as well, it
is a problem as old as the world!

> +void *memset(void *b, int c, unsigned long len)
> +{
> +  unsigned long i;
> +
> +  for (i = 0; i < len; i++)
> +    ((unsigned char *)b)[i] = c;
> +
> +   return b;
> +}
> +
> +/* { dg-final { scan-assembler-not "rlwinm" } } */

Please at least use {\mrlwinm\M}.  There are many other insns that can
be used for this same purpose as well.  We should at least test rldicl
here as well.


Segher