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

[Usman Nadeem via Phabricator via llvm-commits]

mnadeem added a comment.

In D135125#3834198 <https://reviews.llvm.org/D135125#3834198>, @efriedma wrote:

> I'd prefer to try to prevent compile-time explosions in a more reliable way; messing with iteration order doesn't seem like it will work in all cases.

I thought about alternate ways to reduce the compile time but could not come up with anything.

Any ideas are appreciated! Here is snippet of relevant IR https://gist.github.com/UsmanNadeem/5543f14020b654f6c1f1d9dc5dc6ab50 .

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.

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

> messing with iteration order doesn't seem like it will work in all cases.

I agree, but it would be helpful if we could get compile-time data for other benchmarks so that we have the whole picture.
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.

> Even if we do want to mess with the iteration order, just reversing the basic block list isn't really reliable.  See llvm/ADT/PostOrderIterator.h

A lot of tests are throwing `Iterator index out of bound` error with the post order iterator. I think that the JT pass modifies the IR in a way that is messing with the PO iterator. I could break the loop on every change to get around this but it would not be efficient, so not sure if just reversing the list could work here...?

Please let me know your thoughts.


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

https://reviews.llvm.org/D135125