[libc-commits] [PATCH] D149398: [libc] Add support for global ctors / dtors for AMDGPU

[Joseph Huber via Phabricator via libc-commits]

jhuber6 marked an inline comment as done.
jhuber6 added a comment.

In D149398#4307296 <https://reviews.llvm.org/D149398#4307296>, @JonChesterfield wrote:

> Counting on the global looks like a DIY barrier, which is ok, but I can't see anything that stops reordering of operations past the initialisation code run on thread zero.

Obviously we need the DIY barrier because there's no built-in functionality to globally sync on the device. Once the globals have been initialized I simply assume that they'll call main in an orderly fashion and then we wait again at the barrier before finishing.



================
Comment at: libc/startup/gpu/amdgpu/start.cpp:67
+  // initialization code. This will get very, very slow for high thread counts,
+  // but for testing purposes it is unlikely to matter.
+  while (count.load(cpp::MemoryOrder::RELAXED) != get_grid_size())
----------------
sivachandra wrote:
> Can this be avoided at all? As in, if there are globals that have to be initialized on the GPU, then all threads have to wait until they can start using those globals?
Generally I just assume it's unsafe to have any GPU threads calling `main` before we've run all the global constructors. We could reduce this to a regular sync if we placed every global object in thread shared memory however. Then this would be a simple `gpu::sync_threads`. But that would require modifying the source to put `[[clang::addressspace(3)]]` around everything, which is a pretty scarce resource.


================
Comment at: libc/startup/gpu/amdgpu/start.cpp:71
+  gpu::sync_threads();
+}
 
----------------
JonChesterfield wrote:
> This is missing a fence. Noth
The implementation of `sync_threads` has a fence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149398