[libc-commits] [PATCH] D148943: [libc] Enable multiple threads to use RPC on the GPU

[Jon Chesterfield via Phabricator via libc-commits]

JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

I think this is OK. It seems likely that we'll be able to simplify parts of it later. I'll remove the outdated part of the commit message shortly, the state machine has changed since it was written



================
Comment at: libc/src/__support/RPC/rpc.h:106
   /// Initialize the communication channels.
-  LIBC_INLINE void reset(void *lock, void *inbox, void *outbox, void *buffer) {
+  LIBC_INLINE void reset(uint32_t lane_size, void *lock, void *inbox,
+                         void *outbox, void *buffer) {
----------------
Not keen on this being a runtime value but it's not that clear how to avoid that on the host side so this seems OK


================
Comment at: libc/src/__support/RPC/rpc.h:187
     lock[index].fetch_and(and_mask, cpp::MemoryOrder::RELAXED);
+    gpu::sync_lane(lane_mask);
+  }
----------------
a bit frustrated that we need this sync for now


================
Comment at: libc/src/__support/RPC/rpc.h:418
 
-  buffer->opcode = opcode;
+  if (is_first_lane(lane_mask)) {
+    buffer[index].header.opcode = opcode;
----------------
i think we're better off without the is_first_lane here, i.e. let all active lanes write the same value to buffer, but that should only be a codegen improvement


================
Comment at: libc/src/__support/RPC/rpc_util.h:33
 
+/// Get the first active thread inside the lane.
+LIBC_INLINE uint64_t get_first_lane_id(uint64_t lane_mask) {
----------------
weird that these show up in the phab diff, they should already be in trunk


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148943