Better dominators #29140
Better dominators #29140
Conversation
base/compiler/ssair/domtree.jl
Outdated
| function construct_domtree(cfg::CFG) | ||
| idoms = SNCA(cfg) | ||
| nidoms = naive_idoms(cfg) | ||
| @assert idoms == nidoms |
There was a problem hiding this comment.
I'm confused about what's happening here... you construct idoms and nidoms by different algorithms but they must be the same. Is this just here temporarily for debugging?
There was a problem hiding this comment.
Yes the naive way to construct it will go away once I'm convinced it's correct
|
Is DOM still the bottleneck, or did it move to something else? |
|
(numbers are after taking out the old algorithm and assert of course) |
|
Awesome! Did you add more to this branch? I actually gave it whirl yesterday and it was considerably improved, but nowhere near 11s. |
|
Oh, I now see your second comment. That's really great news! |
|
I'm doing a PkgEval run right now (with the assert still in) and plan to merge shortly thereafter if that comes back green (at which point I'll take out the assert also). |
|
I haven't reviewed, but yay! (also, don't forget to drop the "WIP" from the issue title and commit message). |
|
PkgEval came back green. I'll rebase this to remove the assert and remove the WIP. |
This commit replaces the naive algorithm for replacing dominator trees by a faster implementation based on the Semi-NCA algorithm (reference in the code comments). LLVM recently switched to this algorithm and found it to be faster in practice than SLT (which it used before). It is also slightly easier to implement. More importantly though, it should easily extend to dynamic dominators. This fixes the preformance problems in dominator construction noted in #25927 and should provide a basis for a dynamic dominator implementation to fix #29107.
This commit replaces the naive algorithm for replacing dominator
trees by a faster implementation based on the Semi-NCA algorithm
(reference in the code comments). LLVM recently switched to this
algorithm and found it to be faster in practice than SLT (which
it used before). It is also slightly easier to implement. More
importantly though, it should easily extend to dynamic dominators.
I'm hoping this will fix the performance problems with constructing
dominators noted in #25927 as well as providing the basis for a
dynamic dominator implementation to fix #29107.
Current status: Bootstraps fine. Need to run more tests and look at the performance characteristics.