On Wed, 19 Aug 2020, Senthil Kumar Selvaraj wrote: > > Hans-Peter Nilsson writes: > > > On Fri, 14 Aug 2020, Senthil Kumar Selvaraj via Gcc wrote: > >> As you can deduce from the (set_attr "cc" ..), only constraint > >> alternatives 0,2,3 and 6 clobber CC - others leave it unchanged. > > > > Yes, I recognize that. > > > >> My first version of the port adds a post-reload splitter that adds a > >> (clobber (reg:CC REG_CC)) unconditionally, and it appears to work. > > > > Ouch, temporarily lying to gcc here. > > > >> If I > >> do want to add the clobber conditionally, would something like the below > >> be a good way to do it (get_cc_reg_clobber_rtx returns either const0_rtx > >> or cc_reg_rtx based on get_attr_cc (insn))? Or is there a better/cleaner > >> way? > > > > I suggest having a look at what I did for the CRIS port. > > Check the git history. > > > > In short: > > - Add the clobber initially, to *all* patterns. > > - Add postreload splitters for the special combinations that > > don't clobber CC (ones without clobbering CC). > > - Use the old "cc" attribute to control and generate > > clobber/useful CC-setting alternatives (for various new CC_ > > modes) with use of define_subst. > > This sounds like a great plan - the idea of always generating insn > variants for however many CCmodes are available, and then using > cc_enabled to disable them selectively (based on the attribute value > corresponding to the alternative chosen) looks really neat. I did not > understand the purpose of subst_attr for "ccnz" "ccnzvc" and "cccc" > though - wouldn't a (set_attr "cc_enabled", "...") do the same thing?
You mean adding a whole new line with separate per-alternative settings instead of just a suffix to the name, per instruction, to get it compare-elim-optimized? (Ok, plus a tweak to SELECT_CC_MODE; see a33649e). That's not simpler to me, and error-prone without benefits. Note that the "cc" attribute was already there from cc0 times, just as for AVR! But beware, the cc attributes *might* need tweaking, and also note that disabling an alternative for an insn may make gcc pick a subsequent one, which may be invalid when matching operands for the disabled alternative. But that's the simplistic and obvious reply, perhaps I'm misinterpreting and you mean something else? For the purpose of the different CCmodes, see cris-modes.def, in particular the second paragraph, which I'm not repeating here. > Also, I already have a version that hides REG_CC until reload (based on > the suggestion in the wiki page), but I guess this approach will work > equally well with that too? Ow, I see it does! I don't know, that's unchartered territory to me, but it does seem like visium was written that way. It seems a bit wordy though; the define_split isn't necessary if you have the clobber from the start and use define_subst for the other alternatives. You still need splitters for special-cases of insns where condition codes aren't affected though. If these "special cases" approach being the norm, then I don't know. IMHO, introducing REG_CC clobbers only *after* reload seems like it could backfire. Doing things behind gcc's back is what we're eliminating, not re-introducing, to be dogmatic. That wiki page mentions using define_subst to avoid all (near-)duplicates, but not in its examples. brgds, H-P
