On Tue, Aug 18, 2020 at 6:52 AM Senthil Kumar Selvaraj <senthil.theco...@gmail.com> wrote: > > recognize such insns, but as it stands that define_insn would > > recognize the incorrect insn: > > > > [(set (reg:QI 0) (const_int 0)) > > (clobber (scratch:CC))] > > get_cc_reg_clobber_rtx also looks at the insn itself (i.e. whatever > the erstwhile NOTICE_UPDATE_CC hook did), so if the cc attribute is LDI, > and operands[1] is const0_rtx, it would return cc_reg_rtx as the clobber reg. > > AFAICT, some passes after reload verify if operands match constraints, > and that would work in this case - I'm not sure if the pattern you > mentioned could show up, outside of wrong splitters/peepholes in the md file.
I don't think they can, but it's still another case of lying to GCC. At the very least, output_movqi should assert that it isn't asked to produce code for this situation. > Another approach would be to conservatively use a 'c' constraint and > clobber REG_CC for all reg-reg moves. I'd prefer splitting the constraint alternatives to have one clear-r0 alternative, an ldi alternative, and a clear -r1_31 alternative. As for define_subst, is it really worth it? If I'm reading the documentation correctly, it's not powerful enough to deal with scratch operands on its own, so we'd probably need three or four variants of define_subst just to handle those cases. I'm probably missing something obvious, but what's the reason for keeping the CC-clobbering post-reload splitter when we already have a CC-setting one? Are there significant post-reload optimizations that can deal with a clobber but not an arbitrary assignment to CC? If so, wouldn't it be better to fix those optimizations? (There are at least some pre-reload optimizations that don't work with the CC-clobbering insn patterns. lower_subreg.c's simple_move, for example, recognizes only two-operand sets. This resulted in pessimized code, but is easy to fix.)
