Skip to content

Conversation

@al42and
Copy link
Contributor

@al42and al42and commented Sep 25, 2023

The E2E test assumes that the device supports priorities (sm_35 or newer).

The E2E test assumes that the device supports priorities (sm_35 or newer).
@al42and al42and requested review from a team as code owners September 25, 2023 18:15
@al42and al42and temporarily deployed to WindowsCILock September 25, 2023 18:39 — with GitHub Actions Inactive
Copy link
Contributor

@gmlueck gmlueck left a comment

Choose a reason for hiding this comment

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

Spec changes OK.

Copy link
Contributor

@npmiller npmiller left a comment

Choose a reason for hiding this comment

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

LGTM

@steffenlarsen
Copy link
Contributor

@kbenzie | @smaslov-intel - Can me go ahead with review and merge of the adapter changes?

@kbenzie
Copy link
Contributor

kbenzie commented Sep 29, 2023

@kbenzie | @smaslov-intel - Can me go ahead with review and merge of the adapter changes?

We have pulled the CUDA adapter source over to the UR repo already in oneapi-src/unified-runtime#896. #11342 is in progress although its unclear when it will be ready to merge. Soon hopefully, review and GitHub Actions pending.

We can pull over any additional changes to the CUDA adapter source to the UR repo which merges before #11342. I think that's probably the smoothest path forwards in this case if this is ready to go.

Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Looks good!

@steffenlarsen
Copy link
Contributor

@kbenzie | @smaslov-intel - Can me go ahead with review and merge of the adapter changes?

We have pulled the CUDA adapter source over to the UR repo already in oneapi-src/unified-runtime#896. #11342 is in progress although its unclear when it will be ready to merge. Soon hopefully, review and GitHub Actions pending.

We can pull over any additional changes to the CUDA adapter source to the UR repo which merges before #11342. I think that's probably the smoothest path forwards in this case if this is ready to go.

Thank you, @kbenzie ! Let us try and follow that path.

@al42and al42and temporarily deployed to WindowsCILock September 29, 2023 13:28 — with GitHub Actions Inactive
@al42and al42and temporarily deployed to WindowsCILock September 29, 2023 14:04 — with GitHub Actions Inactive
@al42and
Copy link
Contributor Author

al42and commented Sep 29, 2023

I also have plans for doing the same thing with HIP. Would it be better to wait until #11343 and oneapi-src/unified-runtime#903 land, and then do open the pull request to the UR repo?

@kbenzie
Copy link
Contributor

kbenzie commented Oct 2, 2023

I also have plans for doing the same thing with HIP. Would it be better to wait until #11343 and oneapi-src/unified-runtime#903 land, and then do open the pull request to the UR repo?

@al42and yes, that would be ideal, thanks. We plan to move the adapters ASAP. There is a contribtion guide, specifically relevant in this case is the Adpater Change Process.

@steffenlarsen this looks like its ready to merge now.

@steffenlarsen steffenlarsen merged commit 0c33fea into intel:sycl Oct 2, 2023
@al42and al42and deleted the ext-oneapi-priority-cuda branch October 2, 2023 10:48
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.

5 participants