On Mon, Sep 13, 2021 at 05:10:42PM -0500, Peter Bergner wrote: > >> * 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.
These enums are not so very old (from 2010 only). Before that we used define_constant for all. Some backends used separate numbering for unspec and for unspec_volatile, some didn't. The enum scheme supported both the existing practices. It is easier and prettier to have only one namespace for this (when you want to look something up for example, or just reading stuff even). I did however think it quite useful to have the "V" in the name still. But there is nothing forcing us to have any particular naming scheme for the enumeration constants, so that is no blocker :-) Since it is perfectly fine to have multiple define_enum's for the same enumeration, too, converting to this will be pretty easy :-) > >> (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 ??? Ah I see, I looked with my eyes close apparently. "Remove operand 1"? > >> ;; 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. No strong opinion, the existing comment looked out of place, that's all. The latter option adds information, so if you think that is useful to have here, let's go with that? Cheers, Segher
