Hi!

On Mon, Apr 26, 2021 at 01:04:56PM -0500, acsaw...@linux.ibm.com wrote:
> This adds new values for insn attr type for p10 fusion. The genfusion.pl
> script is modified to use them, and fusion.md regenerated to capture
> the new patterns. There are also some formatting only changes to
> fusion.md that apparently weren't captured after a previous commit
> of genfusion.pl.

That is fine, it is a generated file after all.

> @@ -227,7 +229,7 @@ sub gen_2logical
>     ${inner_op} %3,%1,%0\\;${outer_op} %3,%3,%2
>     ${inner_op} %3,%1,%0\\;${outer_op} %3,%3,%2
>     ${inner_op} %4,%1,%0\\;${outer_op} %3,%4,%2"
> -  [(set_attr "type" "logical")
> +  [(set_attr "type" "$fuse_type")
>     (set_attr "cost" "6")
>     (set_attr "length" "8")])
>  EOF

You can make the rest use heredocs as well, when you have some time?  It
is so much easier to read :-)

> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -204,7 +204,9 @@ (define_attr "type"
>     vecsimple,veccomplex,vecdiv,veccmp,veccmpsimple,vecperm,
>     vecfloat,vecfdiv,vecdouble,mtvsr,mfvsr,crypto,
>     veclogical,veccmpfx,vecexts,vecmove,
> -   htm,htmsimple,dfp,mma"
> +   htm,htmsimple,dfp,mma,
> +   fused_arith_logical,fused_cmp_isel,fused_carry,fused_load_cmpi,
> +   
> fused_load_load,fused_store_store,fused_addis_load,fused_mtbc,fused_vector"

Maybe you can document what the fused types are for?  (Maybe we should
for some of the others as well, but most are trivial thankfully.)  You
cannot use inline comments here (we are inside a quoted string), but
maybe you can put something after it?  For example, fused_arith_logical
is not for an arith insn fused with a logical insn, like most others are
patterned like.

Oh, and it doesn't hurt to use more lines here, if you can group things
better that would help.  And of course you can use whitespace before
names here (you cannot tell from existing entries, but :-) )


Okay for trunk, with or without such improvements.  Thanks!


Segher

Reply via email to