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

[Andrew Ng via Phabricator via llvm-commits]

andrewng added a comment.

>> 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.

I don't really have much time right now but I've scanned the change and it LGTM barring this concern over "safety of usage" which I think would be good to have if it can be done relatively easily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142318