[libcxx-commits] [libcxx] [libc++] Optimize `std::exception_ptr` (PR #162773)

[Adrian Vogelsgesang via libcxx-commits]

vogelsgesang wrote:

> Can we split this up [...]?

Happy to split it up to simplify, even if only to ease reviewing 🙂 

I would suggest a slightly different splitting across the commits, though:

1. introduce benchmark
2. change `src/exception_pointer_glibcxx.ipp` to use `_M_{add,release}ref` (still leaving the implementation in the library)
3. expose the selected ABI via `__config_site.in` and change existing code to use it
4. introduce move ctor/assign and possibly swap (the swap may or may not actually be necessary, we'll have to check)
5. inline functions into the header
6. add the `if (ptr_)` fast paths to the inlined functions

Are we aligned at least for steps (1)-(3)? If so I will go ahead and open the corresponding PRs right away

However, I am not sure if splitting (4) and (5) into separate steps is actually a good idea.
Step (4) would introduce new symbols to the library which would not be needed after step (5) anymore.
I guess we should still split it into two separate PRs, but make sure to ship them immediately after each other, so we don't have new symbols in the library?
Or maybe we should swap (4) and (5) around?

> consider whether we can add any attributes to allow improved code gen

I am not quite sure what that step would actually entail. I am not currently aware of any specific attributes which would improve codegen. Do you have any specific attributes in mind?

> do the inlining of the rest of the functions you want, assuming you still think it's worth the cost.

Inlining the destructor, move assignment etc. will be necessary for performance.
Inlining `rethrow_exception` and `current_exception` will not be worth it performance-wise.

I inlined `rethrow_exception` and `current_exception` primarily because
1. it kept the per-ABI code more local / coherent. Instead of having two files per exception ABI (one in `include` and one in `src`), I preferred having a single file per ABI (in `include`)
2. it needed less C++ boilerplate. I only needed the ABI dispatching `#ifdef`s in the `include` directory once instead of twice (once in `include`, once in `src`).

WDYT about those points? Does it make sense to inline `rethrow_exception` and `current_exception` for code simplicity, even if it is not necessary for performance? ("Let's see later, as soon as we reach step (5)" is also a perfectly fine answer for me)

https://github.com/llvm/llvm-project/pull/162773