This is an archive of the discontinued LLVM Phabricator instance.

[libc] Export GPU extensions to `libc` for external use
ClosedPublic

Authored by jhuber6 on Jun 6 2023, 9:13 AM.

Details

Summary

The GPU port of the LLVM C library needs to export a few extensions to
the interface such that users can interface with it. This patch adds the
necessary logic to define a GPU extension. Currently, this only exports
a rpc_reset_client function. This allows us to use the server in
D147054 to set up the RPC interface outside of libc.

Depends on https://reviews.llvm.org/D147054

Diff Detail

Event Timeline

jhuber6 created this revision.Jun 6 2023, 9:13 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 6 2023, 9:14 AM
jhuber6 requested review of this revision.Jun 6 2023, 9:14 AM

@sivachandra This isn't exporting the function rpc_reset_client in the header. Where does that need to be added?

sivachandra added inline comments.Jun 6 2023, 3:11 PM
libc/config/gpu/entrypoints.txt
84

The CMake machinery will look for an entrypoint named reset but more importantly, libc-hdrgen will also look for a function named reset in a spec. Since you do not have a function with name reset, I would have expected libc-hdrgen to error out somewhere. To solve your problem, rename the entrypoint target to match the function name.

libc/src/gpu/CMakeLists.txt
2

We have to match the target name with the entrypoint name.

jhuber6 updated this revision to Diff 529067.Jun 6 2023, 3:43 PM

Change entrypoint to rpc_reset. Still doesn't seem to be appearing in the output function lists.

jhuber6 updated this revision to Diff 529105.Jun 6 2023, 5:25 PM

Exports the function now. Trying ot get it to provide the Opcodes with the enum handling, but it's not showing up.

Exports the function now. Trying ot get it to provide the Opcodes with the enum handling, but it's not showing up.

I suppose you want to add something like:

enum rpc_opcode {
  ...
};

If yes, you will have to add a type header like any of these: https://github.com/llvm/llvm-project/tree/main/libc/include/llvm-libc-types

sivachandra added inline comments.Jun 6 2023, 11:26 PM
libc/spec/gpu_ext.td
19

This is good if you want to add type less enumerations. You will also have to add the list in api.td like this: https://github.com/llvm/llvm-project/blob/main/libc/config/linux/api.td#L149.

jhuber6 updated this revision to Diff 529264.Jun 7 2023, 5:19 AM

Add enumerations for the opcodes and switch over to using the generated header / types file.

sivachandra accepted this revision.Jun 7 2023, 11:16 AM

OK for the file structuring.

libc/include/llvm-libc-types/rpc_opcodes_t.h
12 ↗(On Diff #529264)

This would be C++ - is that OK?

This revision is now accepted and ready to land.Jun 7 2023, 11:16 AM
jhuber6 added inline comments.Jun 7 2023, 11:18 AM
libc/include/llvm-libc-types/rpc_opcodes_t.h
12 ↗(On Diff #529264)

I thought that this was legal C, https://godbolt.org/z/aonTa87Me.

sivachandra added inline comments.Jun 7 2023, 11:33 AM
libc/include/llvm-libc-types/rpc_opcodes_t.h
12 ↗(On Diff #529264)

I thought that this was legal C, https://godbolt.org/z/aonTa87Me.

Using gcc and passing -std=c99 -pedantic shows you this warning:

warning: ISO C does not support specifying 'enum' underlying types before C2X [-Wpedantic]x86-64 gcc (trunk) #1

So, may be OK because it does go through fine with c23. But, we want LLVM's libc to be C11 compliant. Up to you for the GPU side though.

jhuber6 added inline comments.Jun 7 2023, 11:34 AM
libc/include/llvm-libc-types/rpc_opcodes_t.h
12 ↗(On Diff #529264)

I see, well the size is relevant so we could probably do one of those OPCODE_LAST = 0xFFFF things instead.

This revision was landed with ongoing or failed builds.Jun 15 2023, 9:02 AM
This revision was automatically updated to reflect the committed changes.