[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