[PATCH] D58327: [Dominators] Simplify and optimize path compression used in eval()

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar requested changes to this revision.
kuhar added a comment.
This revision now requires changes to proceed.

Hi @MaskRay,

This looks like a fantastic improvement! The only issue is that the code, both original and after your changes, lacks comments.

For example, I can see you put a comment that identified that the second part of the function does path compression, but it's not clear how exactly, and what is the first part of the function doing.

Next, what is the motivation behind this change? Have you identified some bottlenecks in the path compression part, have you suffered form the pathological quadratic complexity of SemiNCA? Perhaps it would make more sense to implement a hybrid algorithm that uses Lengauer-Tarjan for the initial full construction and SemiNCA for incremental updates? I have been thinking about it for quite some time now, but there weren't any complaints about the quadratic complexity yet, thus I figured the additional maintenance cost is not justified.

What was your evaluation for this patch? What benchmarks did you run? How do the numbers look like?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58327