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

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

dexonsmith added a comment.

In D142318#4268765 <https://reviews.llvm.org/D142318#4268765>, @MaskRay wrote:

> In D142318#4268729 <https://reviews.llvm.org/D142318#4268729>, @avl wrote:
>
>> so far I suggest to implement safety check as a separate patch. After having a good solution for this.
>
> Looks good to me.

I think if we don't add the check now it's unlikely to happen later.

In D142318#4268416 <https://reviews.llvm.org/D142318#4268416>, @andrewng wrote:

>>> It also could probably be done in more general way:
>>>
>>>   #if LLVM_ENABLE_ASSERTIONS
>>>   thread_local unsigned threadIndex = -1;
>>>   #else
>>>   thread_local unsigned threadIndex;
>>>   #endif
>>>   inline unsigned getThreadIndex() { 
>>>     assert(threadIndex != -1);
>>>     return threadIndex;
>>>   }
>>
>> However, above suggestion forbids usage of the main thread which is currently used by parallelFor(). Thus the above check either should not be done either parallelFor() should be refactored to not use main thread.
>
> Yes, any checking would need to somehow handle the case where `parallelFor()` falls back to using the main thread or this optimisation would need to be dropped. Also, IIRC, some of the other `parallel` methods may/will still use the main thread concurrently with threads from the thread pool.

Seems like threads are assigned IDs from 1 in the ThreadPoolExecutor constructor via calls to work(). The main thread assigns `threadIndex` to 0 in the same place:

  explicit ThreadPoolExecutor(ThreadPoolStrategy S = hardware_concurrency()) {
    unsigned ThreadCount = S.compute_thread_count();
    // Spawn all but one of the threads in another thread as spawning threads
    // can take a while.
    Threads.reserve(ThreadCount);
    Threads.resize(1);
    std::lock_guard<std::mutex> Lock(Mutex);
    Threads[0] = std::thread([this, ThreadCount, S] {
      for (unsigned I = 1; I < ThreadCount; ++I) {
        Threads.emplace_back([=] { work(S, I); });
        if (Stop)
          break;
      }
      ThreadsCreated.set_value();
      work(S, 0);
    });
  }
  
  void work(ThreadPoolStrategy S, unsigned ThreadID) {
    threadIndex = ThreadID;

Can you explain what will go wrong with the main thread?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142318