Re: [PATCH] rs6000: Disable optimizing multiple xxsetaccz instructions into one xxsetaccz

[Peter Bergner <bergner@linux.ibm.com>]

On 9/12/21 2:26 PM, Segher Boessenkool wrote:
>> I also removed the mma_xxsetaccz define_expand and
>> define_insn_and_split and replaced it with a simple define_insn.
> 
> In the future pleaase do that in a separate patch.  That makes it *much*
> easier to read and review this.

Will do.



>> 	* config/rs6000/mma.md (unspec): Delete UNSPEC_MMA_XXSETACCZ.
>> 	(unspecv): Add UNSPECV_MMA_XXSETACCZ.
> 
> Unrelated to this patch, but I have been wondering this for years:
> should we have an unspecv enum at all?  It causes some churn, and you
> can name the volatile ones UNSPECV_ in either case.

I assumed it was needed, but if not, yeah, one enum would seem to be
better than two.




>> 	(mma_xxsetaccz): Change to define_insn.  Remove match_operand.
>> 	Use UNSPECV_MMA_XXSETACCZ.
> 
> It still has the match_operand.

-(define_insn_and_split "*mma_xxsetaccz"
+(define_insn "mma_xxsetaccz"
   [(set (match_operand:XO 0 "fpr_reg_operand" "=d")
-       (unspec:XO [(match_operand 1 "const_0_to_1_operand" "O")]
-        UNSPEC_MMA_XXSETACCZ))]
+       (unspec_volatile:XO [(const_int 0)]
+                           UNSPECV_MMA_XXSETACCZ))]


It still has "a" match_operand...for operand 0.  The match_operand
for operand 1 was what was removed.  Want me to reword that as
"Remove source match_operand." or "Remove match_operand 1." or ???



>>  ;; We can't have integer constants in XOmode so we wrap this in an UNSPEC.
> 
> Does the comment need updating?  It may help to point out here that itr
>  needs to be volatile.

I think the comment was referring to the unneeded operand which I have
now removed.  I could either remove the comment altogether or change it
to:

;; We can't have integer constants in XOmode so we wrap this in an
;; UNSPEC_VOLATILE.

...to refer to the dummy zero for the source.  Let me know what you want.



>>     (set_attr "length" "4")])
> 
> Not new of course: the default length is 4, most insns have that, it
> helps to be less verbose.

I'll remove that before pushing, thanks!


Peter