Re: [PATCH V5, 2/2] PR target/105325: Fix memory constraints for power10 fusion.

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

On Wed, May 10, 2023 at 11:40:00AM -0400, Michael Meissner wrote:
> This patch applies stricter predicates and constraints for LD and LWA
> instructions with power10 fusion.  These instructions are DS-form instructions,
> which means that the bottom 2 bits of the address must be 0.

The low two bits of the offset, yes.

> --- a/gcc/config/rs6000/genfusion.pl
> +++ b/gcc/config/rs6000/genfusion.pl
> @@ -129,6 +129,12 @@ sub print_ld_cmpi_p10
>    print "  \"\"\n";
>    print "  [(set_attr \"type\" \"fused_load_cmpi\")\n";
>    print "   (set_attr \"cost\" \"8\")\n";
> +
> +  if ($extend eq "sign")
> +    {
> +      print "   (set_attr \"sign_extend\" \"yes\")\n";
> +    }

You never ever need backslashes like this in Perl code, btw.  For
example:
      print qq{   (set_attr "sign_extend" "yes")\n};
or
      print qq/   (set_attr "sign_extend" "yes")\n/;
or
            print <<"HERE"
   (set_attr "sign_extend" "yes")
HERE
or millions of other ways, all of which are much nicer than cramped code
that tries to look like C (but has very different semantics in all ways
that matter).  (Also zillions of ways that are worse still, but that is
the price of freedom maybe :-) )

> -  # Memory predicate to use.
> +  # Memory predicate to use.  For LWA, use the special LWA_OPERAND.

Explain *why*?  It is obvious *what*!

Maybe just split the series into more patches?
> @@ -0,0 +1,26 @@
> +/* { dg-do assemble } */
> +/* { dg-require-effective-target lp64 } */
> +/* { dg-require-effective-target power10_ok } */

power10_ok should no longer exist, btw.  Technical debt has to be
repaid :-/

This patch is readable btw.  Thanks :-)


Segher