[PATCH] D142318: [Support] Add PerThreadBumpPtrAllocator class.

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 13 14:09:44 PDT 2023


avl added a comment.

In D142318#4266388 <https://reviews.llvm.org/D142318#4266388>, @dexonsmith wrote:

> I still have a general concern: this utility isn't safe to use in general LLVM library code, and while that's documented in the header, there's nothing enforcing that or checking for it. I think it'd be easy to get this wrong, and our existing test coverage would be unlikely to catch mistakes, but it could be a big problem for tools/libraries that have their own thread pools and depend on LLVM code.
>
> Is there any way of adding assertions, or a structural change, to confirm this hasn't been used in the wrong place? I think it would be okay to add a bit of API surface to ThreadPoolExecutor, or add some new fields under `LLVM_ENABLE_API_BREAKING_CHECKS`.

the possible solution might be initializing threadIndex to some unrelated value by default.
f.e. setting threadIndex to -1; Threads created by ThreadPoolExecutor would have indexes in range 0 ... ThreadsNum.
It will trigger assertions "assert(getThreadIndex() < NumOfAllocators);" for wrong threads inside PerThreadAllocator methods. Does it sound OK?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142318



More information about the llvm-commits mailing list