[libc] Remove more libc dependencies from the RPC header #116437
+56
−50
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jhuber6
requested review from
JonChesterfield,
lntue,
michaelrj-google and
shiltian
jhuber6
changed the title
Summary: The end goal is to make `rpc.h` a standalone header so that other projects can include it without leaking `libc` internals. I'm trying to replace stuff slowly before pulling it out all at once to reduce the size of the changes. This patch removes the atomic and a few sparse dependencies. Now we mostly rely on the GPU utils, the sleep function, optional, and the type traits. I'll clean these up in future patches. This removed the old stuff I had around the memcpy, but I think that it's not quite as bad as it once was, as it removes a branch and only uses a few extra VGPRs since I believe the builtin memcpy was improved for AMD.
michaelrj-google
approved these changes
Nov 16, 2024
|
|
||
| namespace LIBC_NAMESPACE_DECL { | ||
| namespace rpc { | ||
|
|
||
| /// Conditional to indicate if this process is running on the GPU. | ||
| LIBC_INLINE constexpr bool is_process_gpu() { | ||
| #if defined(LIBC_TARGET_ARCH_IS_GPU) | ||
| #if defined(__NVPTX__) || defined(__AMDGPU__) |
There was a problem hiding this comment.
would it make sense to have a gpu_common.h public header where you could define LIBC_TARGET_ARCH_IS_GPU?
There was a problem hiding this comment.
I think I'm going to cram all of this in rpc_util.h. Question is where I should put those two headers when I'm done such that they can be included by everyone.
There was a problem hiding this comment.
I was assuming they were going in include/gpu/rpc_util.h
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
The end goal is to make
rpc.ha standalone header so that otherprojects can include it without leaking
libcinternals. I'm trying toreplace stuff slowly before pulling it out all at once to reduce the
size of the changes.
This patch removes the atomic and a few sparse dependencies. Now we
mostly rely on the GPU utils, the sleep function, optional, and the
type traits. I'll clean these up in future patches. This removed the old
stuff I had around the memcpy, but I think that it's not quite as bad as
it once was, as it removes a branch and only uses a few extra VGPRs
since I believe the builtin memcpy was improved for AMD.