Skip to content

[libc] Move libc server handlers to a shared header #117908

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 27, 2024
Merged

[libc] Move libc server handlers to a shared header #117908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions libc/shared/rpc_opcodes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#ifndef LLVM_LIBC_SHARED_RPC_OPCODES_H
#define LLVM_LIBC_SHARED_RPC_OPCODES_H

#include "rpc.h"

#define LLVM_LIBC_RPC_BASE 'c'
#define LLVM_LIBC_OPCODE(n) (LLVM_LIBC_RPC_BASE << 24 | n)

Expand Down Expand Up @@ -46,4 +48,12 @@ typedef enum {
RPC_LAST = 0xFFFFFFFF,
} rpc_opcode_t;

#undef LLVM_LIBC_OPCODE

namespace rpc {
// The implementation of this function currently lives in the utility directory
// at 'utils/gpu/server/rpc_server.cpp'.
rpc::Status handle_libc_opcodes(rpc::Server::Port &port, uint32_t num_lanes);
} // namespace rpc

#endif // LLVM_LIBC_SHARED_RPC_OPCODES_H
4 changes: 1 addition & 3 deletions libc/utils/gpu/loader/Loader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
#ifndef LLVM_LIBC_UTILS_GPU_LOADER_LOADER_H
#define LLVM_LIBC_UTILS_GPU_LOADER_LOADER_H

#include "utils/gpu/server/llvmlibc_rpc_server.h"

#include "include/llvm-libc-types/test_rpc_opcodes_t.h"

#include "shared/rpc.h"
Expand Down Expand Up @@ -183,7 +181,7 @@ inline uint32_t handle_server(rpc::Server &server, uint32_t index,
break;
}
default:
status = libc_handle_rpc_port(&*port, num_lanes);
status = handle_libc_opcodes(*port, num_lanes);
break;
}

Expand Down
3 changes: 0 additions & 3 deletions libc/utils/gpu/server/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ target_compile_definitions(llvmlibc_rpc_server PUBLIC
LIBC_NAMESPACE=${LIBC_NAMESPACE})

# Install the server and associated header.
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/llvmlibc_rpc_server.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
COMPONENT libc-headers)
install(TARGETS llvmlibc_rpc_server
ARCHIVE DESTINATION "lib${LLVM_LIBDIR_SUFFIX}"
COMPONENT libc)
24 changes: 0 additions & 24 deletions libc/utils/gpu/server/llvmlibc_rpc_server.h
View file
Open in desktop

This file was deleted.

14 changes: 8 additions & 6 deletions libc/utils/gpu/server/rpc_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
#include "shared/rpc.h"
#include "shared/rpc_opcodes.h"

#include "llvmlibc_rpc_server.h"

#include "src/__support/arg_list.h"
#include "src/stdio/printf_core/converter.h"
#include "src/stdio/printf_core/parser.h"
Expand Down Expand Up @@ -445,15 +443,19 @@ rpc::Status handle_port_impl(rpc::Server::Port &port) {
return rpc::SUCCESS;
}

int libc_handle_rpc_port(void *port, uint32_t num_lanes) {
namespace rpc {
// The implementation of this function currently lives in the utility directory
// at 'utils/gpu/server/rpc_server.cpp'.
rpc::Status handle_libc_opcodes(rpc::Server::Port &port, uint32_t num_lanes) {
switch (num_lanes) {
case 1:
return handle_port_impl<1>(*reinterpret_cast<rpc::Server::Port *>(port));
return handle_port_impl<1>(port);
Copy link
Contributor

@shiltian shiltian Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it confusing looking at the variable name of libc project. what is the rule here?

Copy link
Contributor Author

@jhuber6 jhuber6 Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is code that libc exports to handle the opcodes offload doesn't know how to handle.

case 32:
return handle_port_impl<32>(*reinterpret_cast<rpc::Server::Port *>(port));
return handle_port_impl<32>(port);
case 64:
return handle_port_impl<64>(*reinterpret_cast<rpc::Server::Port *>(port));
return handle_port_impl<64>(port);
default:
return rpc::ERROR;
}
}
} // namespace rpc
5 changes: 1 addition & 4 deletions offload/plugins-nextgen/common/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,7 @@ elseif(${LIBOMPTARGET_GPU_LIBC_SUPPORT})
target_link_libraries(PluginCommon PRIVATE ${llvmlibc_rpc_server})
target_compile_definitions(PluginCommon PRIVATE LIBOMPTARGET_RPC_SUPPORT)
# We may need to get the headers directly from the 'libc' source directory.
target_include_directories(PluginCommon PRIVATE
${CMAKE_SOURCE_DIR}/../libc/utils/gpu/server
${CMAKE_SOURCE_DIR}/../libc/
${CMAKE_SOURCE_DIR}/../libc/include)
target_include_directories(PluginCommon PRIVATE ${CMAKE_SOURCE_DIR}/../libc/)
Copy link
Contributor

@shiltian shiltian Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is fine, but I wonder why don't we just export libc's rpc as a target and then just link against the target, such that CMake will handle all the interface directories.

Copy link
Contributor Author

@jhuber6 jhuber6 Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just described the next patch I'm going to do once this lands. We already have something called include(FindLibcCommonUtils) that I will use after this removes the need for the extra directories.

endif()
endif()

Expand Down
15 changes: 7 additions & 8 deletions offload/plugins-nextgen/common/src/RPC.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

// TODO: This should be included unconditionally and cleaned up.
#if defined(LIBOMPTARGET_RPC_SUPPORT)
#include "llvmlibc_rpc_server.h"
#include "shared/rpc.h"
#include "shared/rpc_opcodes.h"
#endif
Expand Down Expand Up @@ -79,32 +78,32 @@ Error RPCServerTy::runServer(plugin::GenericDeviceTy &Device) {
std::min(Device.requestedRPCPortCount(), rpc::MAX_PORT_COUNT);
rpc::Server Server(NumPorts, Buffers[Device.getDeviceId()]);

auto port = Server.try_open(Device.getWarpSize());
if (!port)
auto Port = Server.try_open(Device.getWarpSize());
if (!Port)
return Error::success();

int Status = rpc::SUCCESS;
switch (port->get_opcode()) {
switch (Port->get_opcode()) {
case RPC_MALLOC: {
port->recv_and_send([&](rpc::Buffer *Buffer, uint32_t) {
Port->recv_and_send([&](rpc::Buffer *Buffer, uint32_t) {
Buffer->data[0] = reinterpret_cast<uintptr_t>(Device.allocate(
Buffer->data[0], nullptr, TARGET_ALLOC_DEVICE_NON_BLOCKING));
});
break;
}
case RPC_FREE: {
port->recv([&](rpc::Buffer *Buffer, uint32_t) {
Port->recv([&](rpc::Buffer *Buffer, uint32_t) {
Device.free(reinterpret_cast<void *>(Buffer->data[0]),
TARGET_ALLOC_DEVICE_NON_BLOCKING);
});
break;
}
default:
// Let the `libc` library handle any other unhandled opcodes.
Status = libc_handle_rpc_port(&*port, Device.getWarpSize());
Status = handle_libc_opcodes(*Port, Device.getWarpSize());
break;
}
port->close();
Port->close();
Copy link
Contributor

@shiltian shiltian Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and then this becomes Port?

NVM. This is offload.


if (Status != rpc::SUCCESS)
return createStringError("RPC server given invalid opcode!");
Expand Down