Hi! On Fri, Mar 22, 2019 at 12:16:30PM -0600, Jeff Law wrote: > So I finally started looking at the fpr-moves regression in this BZ. No > surprise this is further fallout from the combiner changes.
It *exposed* the problem, yes :-) > One might reasonably ask if combine's avoidance of > hard regs should be loosened for consecutive insns -- combining > consecutive insns isn't going to increase the live range of these hard > regs. But that is not the only reason we don't want to forward hard registers. One of the important reasons is correctness: we could create situations that reload/LRA cannot handle (cannot *possibly* handle). Another reason is that combine should not try to do register allocation's job: we on average get better code after the combine hard reg change. (And what are consecutive insns, anyway? This is before scheduling.) > We end up allocating (reg 196) into a GPR. It's "cheaper". Is this a problem in itself already? (I don't know MIPS terribly well). > So one thought was to have a pass of regcprop earlier. That fixes this > MIPS issue nicely *and* simplifies the solution to the fix-r4000 > regression! But regresses x86 in a couple ways. The x86 regressions > can then be fixed by moving the REE pass to just before the early > regcprop pass. That's real promising, so I throw it into the tester and > look for fallout. > > Example fallout on visium where we have this after reload: Yeah, reordering or duplicating late passes is something for stage 1, and even then it might just not work out. > So I'm just punting. In the MIPS splitter, we can peek ahead one real > insn and try to forward propagate the source operand at split time. We > still generate the split insns as well. So after post-reload splitting > we have something like this: > Note how we split the second move too and the source operand is now > $f12. insns 17 and 18 will get removed as dead by DCE and we > ultimately get the desired code. For splitters after reload you have to do a lot of work manually, to get reasonable code. This is a problem everywhere :-( An obvious fix is to split *before* reload, too, but that doesn't work for those few splitters that depend on RA results. Segher