Skip to content
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.

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

Implement multiplexing strategy #638

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Implement multiplexing strategy #638

wants to merge 16 commits into from

Conversation

Kerilk
Copy link
Contributor

@Kerilk Kerilk commented Oct 2, 2023

This implements a multiplexing strategy for HIP similar to Vulkan, where a loader can load installable client driver. It does so by adding a void pointer as the first field of handles, that the loader can use to store it's dispatching information.

Given our virtual classes, objects start by their virtual table, so I use a placement new strategy inside an allocation obtained by malloc. AMD uses a similar strategy in their Vulkan driver.

For more details on the multiplexing strategy please refer to ANL corresponding milestones.

A wrapper library (if it can be cleanly done) would add way more complexity and overhead. We can discuss this in our biweekly as well.

Copy link
Collaborator

@pjaaskel pjaaskel left a comment

Choose a reason for hiding this comment

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

I do not fully follow what is being done in this PR. Can you describe what you exactly plan to do here? Did I understand it right that we want to make L0 and CL pluggable (runtime loadable) so they can be optionally enabled and packaged without needing to have deps for either unless calling them. PoCL has a similar (optional) mechanism in its driver layer. Could we implement it via a plugin layer? That is, add a plugin function which creates the C++ instance of the correct concrete backend class.

void chipstar::Device::QueueDeleter::operator()(
chipstar::Queue *q) const noexcept {
if (q) {
q->~Queue();
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's going on here? Calling constructor directly and freeing the handle. Why not put the freeing to the Queue's destructor and delete it?

@Kerilk
Copy link
Contributor Author

Kerilk commented Oct 3, 2023

I opened this yesterday to discuss what was done with @pvelesko . I should have it labeled draft. I'll elaborate above. The idea is to refactor and incorporate later this year.

@pvelesko pvelesko marked this pull request as draft October 4, 2023 15:35
... without need of placement new and manual destructor
invocation. New approach, applied on chipstar::Queue as demonstration,
discouples the backend from the dispatchable object logic.
@linehill
Copy link
Collaborator

I pushed a commit for proposing an alternate approach that does not need placement new and manual destructor invocation. This is demonstrated with hipStream_t/chipstar::Queue in the commit (a44162c).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants