[libc-commits] [PATCH] D149527: [libc] Support global constructors and destructors on NVPTX

[Artem Belevich via Phabricator via libc-commits]

tra added a subscriber: MaskRay.
tra added inline comments.


================
Comment at: libc/startup/gpu/nvptx/start.cpp:23
+extern "C" {
+// Nvidia's 'nvlink' linker does not provide these sections. We instead need
+// to manually create them and update the globals in the loader implememtation.
----------------
The comment is somewhat puzzling.

The sections themselves would be created by whatever generates the object files, before the linker gets involved.
IIRC from our exchange on discourse, the actual problem was that nvlink discards the sections it's not familiar with and that's why we can't just put the initializers into a known init/fini sections and have to rely on putting initializers among regular data and use explicit symbols to find them.




================
Comment at: libc/test/IntegrationTest/test.cpp:75
+#if defined(__NVPTX__)
+// FIXME: For unknown reason the Nvidia toolchain defines this to be a single
+// byte and will error on a mismatched size otherwise.
----------------
What exactly is the 'toolchain' in this context?


================
Comment at: libc/test/IntegrationTest/test.cpp:80
 // Integration tests are linked with -nostdlib. BFD linker expects
 // __dso_handle when -nostdlib is used.
 void *__dso_handle = nullptr;
----------------
What exactly needs this symbol? 

I'm surprised we need to care about DSOs on NVPTX as we do not have any there.

Googling around (https://stackoverflow.com/questions/34308720/where-is-dso-handle-defined) suggests that we may avoid the issue by compiling with `-fno-use-cxa-atexit`.

@MaskRay -- any suggestions on what's the right way to deal with this?


================
Comment at: libc/utils/gpu/loader/nvptx/CMakeLists.txt:12-13
   CUDA::cuda_driver
+  LLVMObject
+  LLVMSupport
 )
----------------
Can you check how long the clean build of the tool with `-j 6` would take now? If it's in the ballpark of a minute or so, we can probably live with that. Otherwise we should build the tool along with clang/LLVM, similar to how we deal with `libc-hdrgen`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149527