Hi! On Fri, Aug 27, 2021 at 02:58:05PM -0500, Peter Bergner wrote: > Fwprop will happily optimize two xxsetaccz instructions into one xxsetaccz > by propagating the results of the first to the uses of the second. > We really don't want that to happen given the late priming/depriming of > accumulators. I fixed this by making the xxsetaccz source operand an > unspec volatile.
Good good. > 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. > GCC10 suffers from the same issue, but since the code is different, I'll > have to determine a different solution which I'll post as a separate > patch. It doesn't currently have an unspec at all, so you cannot give it a side effect. It shouldn't be hard to make it use an unspec, just a lot of mechanics (and testing :-/ ) > * 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. > (mma_xxsetaccz): Change to define_insn. Remove match_operand. > Use UNSPECV_MMA_XXSETACCZ. It still has the match_operand. > ;; 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. > (set_attr "length" "4")]) Not new of course: the default length is 4, most insns have that, it helps to be less verbose. Okay for trunk with that changelog fix. Thanks! Segher
