On 11/04/2011 05:00 PM, Joern Rennecke wrote: >>> (define_insn_and_split "move_frame" >>> [(set (match_operand:SI 0 "gpr_operand" "=r") >>> (match_operand:SI 1 "register_operand" "r")) >>> (clobber (reg:CC CC_REGNUM))] >>> "operands[1] == frame_pointer_rtx || operands[1] == arg_pointer_rtx" ... >>> (define_insn_and_split "*move_frame_1" >>> [(set (match_operand:SI 0 "gpr_operand" "=r") >>> (match_operand:SI 1 "register_operand" "r")) >>> (clobber (reg:CC CC_REGNUM))] >>> "(reload_in_progress || reload_completed) >>> && (operands[1] == stack_pointer_rtx >>> || operands[1] == hard_frame_pointer_rtx)" >>> "#" >>> "&& 1" >>> [(set (match_dup 0) (match_dup 1))]) ... > The second is for a reload_in_progress / reload_completed frame > pointer reference. I introduced it as a define_insn_and_split at the > same time as in introduced move_frame as an insn pattern.
Look much closer. These patterns are 100% identical. The *move_frame_1 pattern will _never_ be matched, because such an insn will always be matched by move_frame. >> Given that reload *would* generate an add3 (with no clobber), and >> add-without-clobber isn't ordinarily a valid insn, why not just go >> ahead and use that as your reload pattern instead of this >> clobber placeholder? ... > It would stop the pre-reload optimizers of goofing up, but not the > post-reload ones. Fair enough. >> It does seem like you'd do well to split this pattern. If you do, then >> you'll >> automatically get the right changes to debugging unwind info across this. > > The only possible unwind issue I see here is if an interrupt or exception > triggers inside this sequence. And that is assuming that we can unwind > the interrupt / exception otherwise. stepi with gdb was the case I had in mind. >> A test for epilogue_completed in the split condition should be sufficient to >> wait until after peephole2 has finished, so that you don't interfere with the >> transformations you want there. > > Actually, peephole2 runs after thread_prologue_and_epilogue. Yes, but there are no splits between thread_prologue_and_epilogue and peephole2. The first split pass after t_p_a_e is just before sched2. Alpha relies on this distinction already. > For now, I can add a comment about the possible benefit of a splitter. Fair enough. > >> I'm also interested in hearing about how well this whole scheme works in >> practice, as opposed to merely waiting until after reload to split and flags >> users. > > "and flags users"? I think I get the general idea what you are trying to > express, but I can't reconstruct what exactly you wanted to say. On mn10300 (and other targets), cbranch, cstore, adddi3, et al, remain an indivisible pattern until after reload. Thus reload can generate a flags-clobbering addition at any point, because the flags register is never live between insns at that point. After reload, the patterns that require the use of flags are split, both to make it easy to compute instruction sizes and for scheduling. I'm asking what kind of benefits you see from splitting these patterns early, and then working so hard to fix up the problems that might be caused. > They don't know how to put back the hard reg clobbers of > (reg:SF 0) and (reg:SI 1) . Ah, I missed that difference in the patterns. > Actually, it's expand_unop, and it doesn't. Left to its own devices, it > will generate a libcall instead. That's a shame. I wonder how many instances we could fix. Something for a later cleanup, then. r~
