[libc] Add memory fences to device-local locking calls

[JonChesterfield]

Summary

This makes the interface less error prone. The acquire was previously
forgotten. Release is currently missing if recv() is the last operation made
before close.

[JonChesterfield]

• whitespace

[JonChesterfield]

• clang-format

[jhuber6]

libc/src/__support/RPC/rpc.h

188–194

LLVM style has no braces around single line ifs.

531

Why was this removed

[JonChesterfield]

• drop braces around condition

libc/src/__support/RPC/rpc.h

531

Writing the comment out twice seems excessive. This makes this code match the copy&paste in the other class too.

libc/src/__support/RPC/rpc.h

[[clang::convergent]] LIBC_INLINE bool try_lock(uint64_t lane_mask,
     //
     // Cases, per thread:
     // mask==0 -> unspecified before, discarded by ballot -> 0
     // mask==1 and before==0 (success), set zero by ballot -> 0
     // mask==1 and before==1 (failure), set one by ballot -> 1
     //
     // mask != packed implies at least one of the threads got the lock
     // atomic semantics of fetch_or mean at most one of the threads for the lock
-    return lane_mask != packed;
+
+    // If holding the lock then the caller can load values knowing said loads
+    // won't move past the lock. No such guarantee is needed if the lock acquire
+    // failed. This conditional branch is expected to fold in the caller after
+    // inlining the current function.
+    bool holding_lock = lane_mask != packed;
+    if (holding_lock)
+      atomic_thread_fence(cpp::MemoryOrder::ACQUIRE);
+    return holding_lock;
   }
 
   /// Unlock the lock at index. We need a lane sync to keep this function
   /// convergent, otherwise the compiler will sink the store and deadlock.
   [[clang::convergent]] LIBC_INLINE void unlock(uint64_t lane_mask,
                                                 uint64_t index) {
+    // Do not move any writes past the unlock
+    atomic_thread_fence(cpp::MemoryOrder::RELEASE);
+
     // Wait for other threads in the warp to finish using the lock
     gpu::sync_lane(lane_mask);
 
     // Use exactly one thread to clear the bit at position 0 in lock[index]
     // Must restrict to a single thread to avoid one thread dropping the lock,
     // then an unrelated warp claiming the lock, then a second thread in this
     // warp dropping the lock again.
     uint32_t and_mask = ~(rpc::is_first_lane(lane_mask) ? 1 : 0);

 Client::try_open() {
   // Perform a naive linear scan for a port that can be opened to send data.
   for (uint64_t index = 0; index < this->port_count; ++index) {
     // Attempt to acquire the lock on this index.
     uint64_t lane_mask = gpu::get_lane_mask();
     if (!this->try_lock(lane_mask, index))
       continue;
 
-    // The mailbox state must be read with the lock held.
-    atomic_thread_fence(cpp::MemoryOrder::ACQUIRE);
-
     uint32_t in = this->load_inbox(index);
     uint32_t out = this->load_outbox(index);
 
     // Once we acquire the index we need to check if we are in a valid sending
     // state.
     if (this->buffer_unavailable(in, out)) {
       this->unlock(lane_mask, index);
       continue;
for (uint64_t index = 0; index < this->port_count; ++index) {
 
     // The server is passive, if there is no work pending don't bother
     // opening a port.
     if (this->buffer_unavailable(in, out))
       continue;
 
     // Attempt to acquire the lock on this index.
     uint64_t lane_mask = gpu::get_lane_mask();
-    // Attempt to acquire the lock on this index.
     if (!this->try_lock(lane_mask, index))
       continue;
 
-    // The mailbox state must be read with the lock held.
-    atomic_thread_fence(cpp::MemoryOrder::ACQUIRE);
-
     in = this->load_inbox(index);
     out = this->load_outbox(index);
 
     if (this->buffer_unavailable(in, out)) {
       this->unlock(lane_mask, index);
       continue;
     }