[PATCH] D71548: Fix time trace multi threaded support with LLVM_ENABLE_THREADS=OFF

[Russell Gallop via Phabricator via llvm-commits]

russell.gallop added a comment.

In D71548#1787415 <https://reviews.llvm.org/D71548#1787415>, @teemperor wrote:

> I don't think `LLVM_THREAD_LOCAL` is correct here. unique_ptr isn't a POD so this doesn't really fit to the use case of` LLVM_THREAD_LOCAL`. Maybe we should have a `LLVM_NONTRIVIAL_THREAD_LOCAL`. that doesn't fall back to `__thread`. Or we just use raw pointer here instead of a global unique_ptr (it seems that we anyway use it here just as an annotation to describe memory ownership?).


I'd prefer a "thread_local if threads enabled" as the unique_ptr helps enforce ownership when https://reviews.llvm.org/D71060 lands.

Interestingly, thread_local was okay on all public bots, the only problem was with LLVM_ENABLE_THREADS=OFF so maybe LLVM_THREAD_LOCAL could be simplified to "thread_local if threads enabled" now (removing __declspec(thread) and __thread). I don't know enough about how that is used to know whether that would be okay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71548