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

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

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

> In D142318#4270235 <https://reviews.llvm.org/D142318#4270235>, @dexonsmith wrote:
>
>> In D142318#4269744 <https://reviews.llvm.org/D142318#4269744>, @avl wrote:
>>
>>> 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.
>
> This check will help for pure users of getThreadIndex() but will not help users of PerThreadBumpPtrAllocator as it calls "detail::Executor::getDefaultExecutor()->getThreadsNum();" in the constructor. Thus any call to getThreadIndex() after PerThreadBumpPtrAllocator is created will have HasDefaultExecutor == true.

I see; I've been missing that detail.

You could introduce a free function, `getDefaultThreadsNum()` to Parallel.h/cpp, which in assertions mode calls `hardware_concurrency().compute_thread_count()` (redundantly) without calling `getDefaultExecutor()`. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142318