[PATCH] D135125: [JumpThreading] Reverse the order of basic block iteration.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> When JT is called on ._crit_edge2292, %uglygep.13072 is replicated in the threaded block and a PHI node (lets say %phi.1) is inserted into the successor i.e. ._crit_edge2292.1. In the next iteration JT is called on ._crit_edge2292.1 and we now need to build two PHI nodes when rewriting the SSA. One for the new value %uglygep.23073 and one for %phi.1 that we created earlier. The number of new phi nodes we need to build keeps adding up as we go down the chain.

I'm not sure I understand the growth aspect here; %uglygep.13072 isn't defined or used in any of the blocks that are getting replicated?  Or is the issue that the SSA algorithm spends time trying to rewrite things that don't need to be rewritten?  Would using SSAUpdaterBulk in JumpThreadingPass::updateSSA help?

> If we iterate from the bottom up then then we do not have this issue by design.

Not this exact issue, no, but trying to fix a local issue by making a global change tends to show other algorithmic weaknesses.

> A lot of tests are throwing Iterator index out of bound error with the post order iterator.

I think the post order iterator caches some data; probably if JumpThreading is modifying blocks while you iterate, that won't work. Maybe you could use the iterator to construct a worklist or something like that.

> On another note, I did run a performance experiment and did not see any regression and saw a perf 3% gain in SPEC2017/xz. And look at the test output changes I do see some more threading.

That might be an argument for changing the iteration order, if we can explain why it's happening.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135125/new/

https://reviews.llvm.org/D135125