[PATCH] D53245: Teach the DominatorTree fallback to recalculation when applying updates to speedup JT (PR37929)

[Chijun Sima via Phabricator via llvm-commits]

NutshellySima added a comment.

In https://reviews.llvm.org/D53245#1275891, @brzycki wrote:

> > The issue I find with cl::opt here is that this is a generic class that may not necessarily have a corresponding cpp file -- there can be multiple clients to GenericDomTreeConstruction, each with different requirements. Because of that, it's not clear to me in which translation unit one would put cl::opt for the constant in.
> >  Is that a solved problem somewhere else in LLVM? I'm not familiar with using cl::opt in header files.
>
> Fair enough. I'm more used to working on function passes that have several `cl::opt()` lines at the top of the .cpp file. If we don't have a corresponding cpp it's non-trivial to add it as a command-line argument. I too have never tried to use `cl::opt()` in a .h.


I'm also not familiar with `cl::opt()` in headers and I think adding a function parameter is an option.

The constant "40" selected is based on the previous table and additional tests on "25" and "50" (see below),

| α           | y/clang | y/sqlite | y/oggenc |
| 0/baseline  | 33.20%  | 25.60%   | 28.40%   |
| 20          | 20.70%  | 21.70%   | 22.50%   |
| 25          | 20.40%  | ---      | ---      |
| 30          | 13.10%  | 23.10%   | 23.70%   |
| 40          | 13.60%  | 24.10%   | 24.90%   |
| 50          | ---     | 25.60%   | ---      |
| 55          | 14.20%  | 26.00%   | 28.00%   |
| 40 (n>=100) | 13.30%  | 21.20%   | 24.50%   |
|

I'll test `n>=200/300/400` to see whether it is better.


Repository:
  rL LLVM

https://reviews.llvm.org/D53245