[PATCH] D53245: Teach the DominatorTree fallback to recalculation when applying updates to speedup JT (PR37929)
Chijun Sima via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 25 09:45:14 PDT 2018
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
More information about the llvm-commits
mailing list