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
