[PATCH] D58373: [Dominators] Avoid potentially quadratic std::is_permutation

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 27 07:33:03 PST 2019


kuhar added a comment.

In D58373#1406843 <https://reviews.llvm.org/D58373#1406843>, @MaskRay wrote:

> In D58373#1402235 <https://reviews.llvm.org/D58373#1402235>, @kuhar wrote:
>
> > `Roots.size()` is only > 1 for the PostDominatorTree, and I'd expect it to be small (< 100) on virtually all functions. `is_permulation` was used in the original code because it doesn't allocate and should be cheaper for small trees.
> >
> > Do you have some statistics on the size of Roots? At what size does SmallPtrSet start to outperform `is_permuatation`?
>
>
> Thanks for the empirical data (<100). I find SmallPtrSet<void *, 4> starts to outperform `std::is_permutation` when there are about 16 elements on my machine. It may be 2x slower (should be larger, but I put it in a loop and the loop itself has some costs) when there is only one root. So the benefit to use a `SmallPtrSet` here is just to filter out some unlikely cases (>1000 roots) and to clear up my doubts when I see that `verifyRoots` claims to be O(N)...


It seems it would be best to create a small helper function that does permutation check using std::is_permuatation for small arrays and uses SmallPtrSet for the rest.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58373





More information about the llvm-commits mailing list