On Thu, Nov 21, 2019 at 08:32:14PM +0000, Richard Sandiford wrote: > Segher Boessenkool <seg...@kernel.crashing.org> writes: > > It's not great if every pass invents its own version of some common > > infrastructure thing because that common one is not suitable. > > > > I.e., can this be fixed somehow? Maybe just by having a restricted DU > > chains df problem? > > Well, it'd probably make sense to make fwprop.c's approach available > as a "proper" df interface at some point. Hopefully if anyone wants the > same thing as fwprop.c, they'd do that rather than copy the code. :-)
> >> * Updating a full, ordered def-use chain after a move is a linear-time > >> operation, so whatever happens, we'd need to apply some kind of limit > >> on the number of uses we maintain, with something like that integer > >> point range for the rest. Yeah. > >> * Once we've analysed the insn and built its def-use chains, we don't > >> look at the df_refs again until we update the chains after a successful > >> combination. So it should be more efficient to maintain a small array > >> of insn_info_rec pointers alongside the numerical range, rather than > >> walk and pollute chains of df_refs and then link back the insn uids > >> to the pass-local info. > > > > So you need something like combine's LOG_LINKS? Not that handling those > > is not quadratic in the worst case, but in practice it works well. And > > it *could* be made linear. > > Not sure why what I've used isn't what I need though :-) I am wondering the other way around :-) Is what you do for combine2 something that would be more generally applicable/useful? That's what I'm trying to find out :-) What combine does could use some improvement, if you want to hear a more direct motivations. LOG_LINKS just skip references we cannot handle (and some more), so we always have to do modified_between etc., which hurts. > >> Target Tests Delta Best Worst Median > >> avr-elf 1341 -111401 -13824 680 -10 > > > > Things like this are kind of suspicious :-) > > Yeah. This mostly seems to come from mopping up the extra moves created > by make_more_copies. So we have combinations like: > > 58: r70:SF=r94:SF > REG_DEAD r94:SF > 60: r22:SF=r70:SF > REG_DEAD r70:SF Why didn't combine do this? A target problem? > So there's only one case in which it isn't a win, but the number of > tests is tiny. So I agree there's no justification for trying this in > combine proper as things stand (and I wasn't arguing otherwise FWIW). > I'd still like to keep it in the new pass because it does help > *sometimes* and there's no sign yet that it has a noticeable > compile-time cost. So when does it help? I can only think of cases where there are problems elsewhere. > It might also be interesting to see how much difference it makes for > run-combine=4 (e.g. to see how much it makes up for the current 2-insn > limit)... Numbers are good :-) Segher