[clang] [llvm] DO NOT MERGE: Identify places that need reserve. (PR #136543)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 21 01:01:47 PDT 2025
kazutakahirata wrote:
@kuhar I understand that you are working on improving the performance of `llvm::append_range`.
I did some experiment with this PR:
- I taught `llvm::append_range` to:
- automatically call `Container::reserve` if the given range is a pointer or `std::random_access_iterator_tag`, and
- crash on capacity changes after calling `Container::insert`.
- I built a debug version of compiler (for clean stack traces).
- I used the debug version of the compiler to build an optimized version of clang while manually adding `reserve` to fix crashes.
One noteworthy point is that I only needed to add a dozen or so calls to `reserve`. Some ranges are of sets like `SmallPtrSet`, so `size()` is immediately available. We might even upstream those calls. Others require linear walk, so we shouldn't upstream those.
I'm wondering if porting those `reserve()` calls on `SmallPtrSet` and such to your patch (https://github.com/llvm/llvm-project/pull/136365) might improve the performance. Also, I am wondering if your patch needs to deal with pointer ranges like `char *`, which I assume doesn't have an iterator category. Note that `ArrayRef` uses pointers as iterator types.
We could detect the presence of `size()` on ranges and use that as a hint when given ranges are neither pointer nor `std::random_access_iterator_tag`. Alternatively, we could manually add calls to `reserve()` just before `append_range` where ranges come with `size()`, which we could detect with some temporary template meta-programming hacks to `append_range`.
https://github.com/llvm/llvm-project/pull/136543
More information about the llvm-commits
mailing list