This is an archive of the discontinued LLVM Phabricator instance.

[libc] Remove `MAX_LANE_SIZE` definition from the RPC server
ClosedPublic

Authored by jhuber6 on Aug 23 2023, 9:10 AM.

Details

Summary

This MAX_LANE_SIZE was a hack from the days when we used a single
instance of the server and had some GPU state handle it. Now that we
have everything templated this really shouldn't be used. This patch
removes its use and replaces it with template arguments.

Diff Detail

Event Timeline

jhuber6 created this revision.Aug 23 2023, 9:10 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 23 2023, 9:10 AM
jhuber6 requested review of this revision.Aug 23 2023, 9:10 AM
jhuber6 updated this revision to Diff 552749.

Fix comment

I'm not sure about this being on the methods. Why not the class itself? We have a bad time if different places manage to disagree on the value

libc/utils/gpu/loader/Loader.h
114

Static assert that it's 32 or 64 here, or maybe go so far as a valid-for-target call?

libc/utils/gpu/loader/amdgpu/Loader.cpp
425

handle_error if it isn't 32 or 64 please. Maybe a switch on size with handle_error in the default?

jhuber6 updated this revision to Diff 552752.Aug 23 2023, 9:19 AM

Address comments. The template is on the handle_server call because it's a variant class we use std::visit on.

libc/utils/gpu/loader/Loader.h
112

This will never pass

jhuber6 updated this revision to Diff 552772.Aug 23 2023, 9:57 AM

Oops, wrong conditional.

libc/utils/gpu/server/rpc_server.cpp
96

Was a functional change intended here?

jhuber6 added inline comments.Aug 23 2023, 9:59 AM
libc/utils/gpu/server/rpc_server.cpp
96

This isn't technically a functional change, since this callback automatically uses the current thread mask I could put this part in here instead which serves the purpose of getting rid of the invocation of the lane size.

JonChesterfield accepted this revision.Aug 23 2023, 10:00 AM
This revision is now accepted and ready to land.Aug 23 2023, 10:00 AM
This revision was landed with ongoing or failed builds.Aug 23 2023, 10:10 AM
This revision was automatically updated to reflect the committed changes.