[PATCH] D72399: [Support] Replace Windows __declspec(thread) with thread_local in LLVM_THREAD_LOCAL

Wed Jan 15 03:15:05 PST 2020

russell.gallop marked an inline comment as done.
russell.gallop added a comment.

In D72399#1820866 <https://reviews.llvm.org/D72399#1820866>, @rnk wrote:

> lgtm
>
> I checked that `__declspec(thread)` and `thread_local` seem to be functionally equivalent with MSVC.

Thanks.

> Honestly, if `__thread` is available, I wonder if LLVM should prefer that, since it implements the check that the variable is trivially constructible/destructible. But, that has no bearing on which declspec to use.

Is "trivially constructible/destructible" actually a requirement for this? I thought that restriction was due to host platforms which didn't support C++11 thread_local (hence if/when every platform supported C++11 thread_local then the restriction would go away).

I want something (for D71059 <https://reviews.llvm.org/D71059>) which will use C++ thread_local if LLVM_ENABLE_THREADS is enabled but doesn't need to enforce trivially constructible/destructible as long as host tools support is there. If this macro is specifically designed for more limited uses, maybe I need something slightly different that prefers thread_local (over __thread).

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

https://reviews.llvm.org/D72399