[PATCH] D73508: [LLD][COFF] Fix dll import for thread_local storage

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

FWIW, it seems like BUILD_SHARED_LIBS is explicitly disallowed in MSVC configs: https://github.com/llvm/llvm-project/blob/master/llvm/CMakeLists.txt#L555-L561

However people do seem to be building LLVM with this option for mingw configs: https://github.com/msys2/MINGW-packages/pull/6098 But do note that they're setting `--allow-multiple-definition` to avoid issues with multiple definitions (which was the issue I remember running into); I'm not entirely convinced setting that option is a good idea, but apparently it works well enough for them. If you have time to spare, looking into that and trying to avoid it properly would be appreciated.

So in general BUILD_SHARED_LIBS should work for mingw, at least it is not very far from working, so it's probably better to spend effort on making it work than to explicit forbid it.

Regarding the thread_local variable, I think the absolute safest thing to do would be to fix llvm/Support/TimeProfiler.h to only access the TimeTraceProfilerInstance variable via getter/setter functions (defined in lib/Support/TimeProfiler.cpp); then it should work fine across DLL boundaries and would avoid a lot of tricky TLS cases. Or instead of two getter/setter functions, just a wrapper function that returns a pointer. Then it could even be made transparent like this:

  #ifdef _WIN32
  // Actual LLVM_THREAD_LOCAL TimeTraceProfiler * variable is defined as static in TimeProfiler.cpp
  TimeTraceProfiler **getTimeTraceProfilerInstancePtr();
  #define TimeTraceProfilerInstance (*getTimeTraceProfilerInstancePtr())
  #else
  extern LLVM_THREAD_LOCAL TimeTraceProfiler *TimeTraceProfilerInstance;
  #endif

(Not sure if such define trickery is appreciated, or if it's better to just rewrite the code in TimeProfiler.h to unconditionally use that wrapper function.)


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D73508