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

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 14 16:36:32 PDT 2023


dexonsmith added a comment.

In D142318#4269744 <https://reviews.llvm.org/D142318#4269744>, @avl wrote:

> In D142318#4269662 <https://reviews.llvm.org/D142318#4269662>, @dexonsmith wrote:
>
>> In D142318#4269644 <https://reviews.llvm.org/D142318#4269644>, @avl wrote:
>>
>>> I am OK to do that separate patch right after the current patch. Just do not have a good idea for this at the moment.
>>
>> WDYT of the idea above, to have a Boolean flag that checks whether `getDefaultExecutor()` has been called, and assert on that in `getThreadIndex()`?
>
> I think this solves only part of the problem: it checks the fact that executor is already created when getThreadIndex() is requested. But it does not check that thread index is valid. If thread was created not by ThreadPoolExecutor then it would have zero index which clashes with thread index of main thread and Thread0. I thought we want to check that other threads were not used with getThreadIndex.
>
> Checking ThreadPoolExecutor existence still useful check and it would be good to implement it. If we found a good way to check thread indexes it would also be useful.

Yeah, seems like a good start for now. This would catch the case where someone is NOT using `llvm::parallel` at all, but has a bunch of threads, and is wrongly assuming this allocator is safe for concurrent use in general.

Checking the thread indexes seems hard, since the "main" thread could be a different client thread on different entries to `llvm::parallel`.


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