[libc-commits] [PATCH] D150365: [libc][NFC] Simplifly inbox and outbox state handling

[Joseph Huber via Phabricator via libc-commits]

jhuber6 updated this revision to Diff 522260.
jhuber6 added a comment.

Update to make this more obvious that we are simply moving the inversion from
the load to the condition. The inversion affects the condition so it is more
intuitive for the condition itself to be inverted. That is `!a == b` is the same
as `!(a == b)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150365

Files:
  libc/src/__support/RPC/rpc.h


Index: libc/src/__support/RPC/rpc.h
===================================================================
--- libc/src/__support/RPC/rpc.h
+++ libc/src/__support/RPC/rpc.h
@@ -78,27 +78,18 @@
 constexpr uint64_t DEFAULT_PORT_COUNT = 64;
 
 /// A common process used to synchronize communication between a client and a
-/// server. The process contains an inbox and an outbox used for signaling
-/// ownership of the shared buffer between both sides.
+/// server. The process contains a read-only inbox and a write-only outbox used
+/// for signaling ownership of the shared buffer between both sides. We assign
+/// ownership of the buffer to the client if the inbox and outbox bits match,
+/// otherwise it is owned by the server.
 ///
-/// No process writes to its inbox. Each toggles the bit in the outbox to pass
-/// ownership to the other process.
-/// When inbox == outbox, the current state machine owns the buffer.
-/// Initially the client is able to open any port as it will load 0 from both.
-/// The server inbox read is inverted, so it loads inbox==1, outbox==0 until
-/// the client has written to its outbox.
-///
-/// This process is designed to support mostly arbitrary combinations of 'send'
-/// and 'recv' operations on the shared buffer as long as these operations are
-/// mirrored by the other process. These operations exchange ownership of the
-/// fixed-size buffer between the users of the protocol. The assumptions when
-/// using this process are as follows:
-///   - The client will always start with a 'send' operation
-///   - The server will always start with a 'recv' operation
-///   - For every 'send' / 'recv' call on one side of the process there is a
-///     mirrored 'recv' / 'send' call.
-///
-template <bool InvertInbox> struct Process {
+/// This process is designed to allow the client and the server to exchange data
+/// using a fixed size packet in a mostly arbitrary order using the 'send' and
+/// 'recv' operations. The following restrictions to this scheme apply:
+///   - The client will always start with a 'send' operation.
+///   - The server will always start with a 'recv' operation.
+///   - Every 'send' or 'recv' call is mirrored by the other process.
+template <bool Invert> struct Process {
   LIBC_INLINE Process() = default;
   LIBC_INLINE Process(const Process &) = delete;
   LIBC_INLINE Process &operator=(const Process &) = delete;
@@ -148,23 +139,12 @@
                                  alignof(Packet))));
   }
 
-  /// Inverting the bits loaded from the inbox in exactly one of the pair of
-  /// processes means that each can use the same state transitions.
-  /// Whichever process has InvertInbox==false is the initial owner.
-  /// Inbox equal Outbox => current process owns the buffer
-  /// Inbox difer Outbox => current process does not own the buffer
-  /// At startup, memory is zero initialised and raw loads of either mailbox
-  /// would return zero. Thus both would succeed in opening a port and data
-  /// races result. If either inbox or outbox is inverted for one process, that
-  /// process interprets memory as Inbox!=Outbox and thus waits for the other.
-  /// It is simpler to invert reads from the inbox than writes to the outbox.
+  /// Retrieve the inbox state from memory shared between processes.
   LIBC_INLINE uint32_t load_inbox(uint64_t index) {
-    uint32_t i = inbox[index].load(cpp::MemoryOrder::RELAXED);
-    return InvertInbox ? !i : i;
+    return inbox[index].load(cpp::MemoryOrder::RELAXED);
   }
 
   /// Retrieve the outbox state from memory shared between processes.
-  /// Never needs to invert the associated read.
   LIBC_INLINE uint32_t load_outbox(uint64_t index) {
     return outbox[index].load(cpp::MemoryOrder::RELAXED);
   }
@@ -179,9 +159,12 @@
     return inverted_outbox;
   }
 
-  /// Determines if this process needs to wait for ownership of the buffer.
+  /// Determines if this process needs to wait for ownership of the buffer. We
+  /// invert the condition on one of the processes to indicate that if one
+  /// process owns the buffer then the other does not.
   LIBC_INLINE static bool buffer_unavailable(uint32_t in, uint32_t out) {
-    return in != out;
+    bool cond = in != out;
+    return Invert ? !cond : cond;
   }
 
   /// Attempt to claim the lock at index. Return true on lock taken.
@@ -279,12 +262,12 @@
 
   /// Offset of the inbox in memory. This is the same as the outbox if inverted.
   LIBC_INLINE static uint64_t inbox_offset(uint64_t port_count) {
-    return InvertInbox ? mailbox_bytes(port_count) : 0;
+    return Invert ? mailbox_bytes(port_count) : 0;
   }
 
   /// Offset of the outbox in memory. This is the same as the inbox if inverted.
   LIBC_INLINE static uint64_t outbox_offset(uint64_t port_count) {
-    return InvertInbox ? 0 : mailbox_bytes(port_count);
+    return Invert ? 0 : mailbox_bytes(port_count);
   }
 
   /// Offset of the buffer containing the packets after the inbox and outbox.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150365.522260.patch
Type: text/x-patch
Size: 4968 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20230515/7e3a1747/attachment-0001.bin>