[libc-commits] [PATCH] D149598: [libc] Support concurrent RPC port access on the GPU

[Joseph Huber via Phabricator via libc-commits]

jhuber6 added inline comments.


================
Comment at: libc/src/__support/RPC/rpc.h:353
+  // Perform a naive linear scan for a port that can be opened to send data.
+  for (uint64_t index = 0; index < port_size; ++index) {
+    // Attempt to acquire the lock on this index.
----------------
JonChesterfield wrote:
> Your data representation induces overhead here.
> 
> inbox.load hits the bus to retrieve one bit of information. If it fails, next step through the loop starts over from scratch.
> 
> If you change to packed bits, the first atomic load gets you the state of ~32 mailboxes in one tick. If your current index doesn't work out, you now have enough information to pick the next credible candidate without hitting the bus at all.
> 
> Also you're loading from the outbox despite the value of that being known already. Maybe the latency on that redundant load is hidden by the latency on loading from the inbox.
I can see the performance benefits of bit-packing here I'd assume unless the RPC client is mostly unused the performance penalty of false sharing more atomic indices will be small compared to saving several atomic checks in the client / server. We can also definitely hoist the outbox load.


================
Comment at: libc/src/__support/RPC/rpc.h:355
+    // Attempt to acquire the lock on this index.
+    uint64_t lane_mask = gpu::get_lane_mask();
+    uint32_t previous = 0;
----------------
JonChesterfield wrote:
> You could make this less vulnerable to data races on volta by getting the appropriate thread mask outside of the loop. That's also likely to be faster on amdgpu.
Good point, since this is marked convergent it's unlikely that the compiler will remove this from the loop itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149598