This is an archive of the discontinued LLVM Phabricator instance.

[libc][rpc] Allocate a single block of shared memory instead of three
ClosedPublic

Authored by JonChesterfield on May 10 2023, 6:13 PM.

Details

Summary

Allows moving the pointer swap between server and client into reset.
Single allocation simplifies whatever allocates the client/server, currently
the libc loaders.

Diff Detail

Event Timeline

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 10 2023, 6:13 PM
JonChesterfield requested review of this revision.May 10 2023, 6:13 PM
JonChesterfield edited the summary of this revision. (Show Details)May 10 2023, 6:17 PM

constexpr on the functions allowed ad hoc testing along the lines of:

enum {N = 128};
struct equiv {
  atomic<uint32_t> primary[N];
  atomic<uint32_t> secondary[N];
  Packet buffer[N];
};
static_assert(Process<false>::memory_offset_primary_mailbox(N) == __builtin_offset(equiv, primary));

but I can't see an obvious place to check in tests like that

jhuber6 added inline comments.May 10 2023, 6:37 PM
libc/src/__support/RPC/rpc.h
122

Static cast

122

This is unrelated, but I think we might want to call InvertInbox something more descriptive in the fact that it's the server / client.

130
273

Not sure we need to bother with the macro, I'm sure the optimizer can handle forwarding 32 or __AMDGCN_WAVEFRONTSIZE.

libc/src/__support/RPC/rpc_util.h
54 ↗(On Diff #521189)

Did you format this with a different style?

libc/startup/gpu/amdgpu/start.cpp
41

I'd like something more like rpc_shared_buffer or something so we know what it's for.

libc/src/__support/RPC/rpc.h
273

gone with this so it matches the definition of the payload type. alternative is something like sizeof(header) + lane_size*N which seems more obscure

libc/src/__support/RPC/rpc_util.h
54 ↗(On Diff #521189)

no, adding constexpr hit a line limit. can drop constexpr across the board from this patch if necessary

  • rename variable, drop constexpr in various places
  • comma requested
JonChesterfield marked 3 inline comments as done.May 10 2023, 6:59 PM
jhuber6 accepted this revision.May 10 2023, 7:00 PM

We can clean some of this stuff up later. Thinking about doing a big NFC for comments and other things.

This revision is now accepted and ready to land.May 10 2023, 7:00 PM