On Sat, Feb 28, 2015 at 5:38 PM, Georg-Johann Lay wrote: > Am 02/26/2015 um 11:45 PM schrieb Steven Bosscher: >> >> On Thu, Feb 26, 2015 at 8:35 PM, Georg-Johann Lay wrote: >>> >>> Take #2 introduces a new, avr-specific rtl pass whose sole purpose is to >>> rectify notes. The pass is scheduled right before cfg does down (right >>> before .*free_cfg) so that cfg and hence df machinery is available. >>> >>> Regression tests look fine and for the test case the patches produce >>> correct >>> code and correct insn length. >> >> >> Sorry for party-pooping, but it seems to me that the real bug is that >> the target is lying to reload. >> >> IIUC the AVR port selects the insn alternative after register >> allocation (after reload). It bases its selection on REG_DEAD notes. >> In PR64331 an alternative is used that clobbers r20 that has a >> REG_DEAD note, but r20 is not actually dead because hardreg-cprop has >> propagated it forward without adjusting the note. > > > It's not actually about constraint alternatives. > > Let me give an example: Testing HI for 0. The usual sequence would be > > cc0 = reg.low == 0 > cc0 = cc0 && reg.high == 0 > > which costs 2 instructions. If reg is unused after, then ORing can be used > and cc0 will be set as a side effect. This costs 1 insn: > > cc0 = (reg.low |= reg.high) > > Using alternatives would double their number, i.e. 14 instead of 7 for > *cmphi. > > These constraint alternatives would have to express > > 1) reg-alloc, please, use alternative #1 (with clobber of reg) > if the register is unused after > > 2) reg-alloc, please, use alternative #2 (not clobbering reg) > if the register is used after > > If we assume for a moment that we have a *testhi insn and the old *tsthi had > just 1 alternative: > > > (define_insn "*tsthi" > [(set (cc0) > (compare > (match_operand:ALL2 0 "register_operand" "r") > (const_int 0)))] > ...) > > > Then the new one would be something like > > > (define_insn "*tsthi" > [(set (cc0) > (compare > (match_operand:ALL2 0 "register_operand" "r,r") > (const_int 0))) > (clobber (match_scratch:HI 1 "=0,X")] > ...) > > But how can I express 1) and 2) ?
I don't think you can. Other ports express such transformations using define_peephole2 and peep2_reg_dead_p. > Currently, my preferred approach is a new drop-in replacement for the old > reg_unused_after which uses clobbers to decide whether or not op 0 is still > needed. That way, reg-alloc can work like before and there is no need to > implement dozens of new constraint alternatives across the md files. The problem with this approach is that it may break at random. There's just no guarantee that it will work, because you're relying on information that just isn't valid anymore. Unfortunately I don't know enough about CC0-targets to suggest an alternative. Ciao! Steven
