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.

Sign up for GitHub

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

Jump to bottom

[Bindless][Exp] Rename interop related structs/funcs with "external" #1819

Merged
merged 3 commits into from
Jul 29, 2024
Merged

[Bindless][Exp] Rename interop related structs/funcs with "external" #1819

merged 3 commits into from
Jul 29, 2024

Conversation

DBDuncan
Copy link
Contributor

@DBDuncan DBDuncan commented Jul 4, 2024

Rename interop related structs, functions, and enums with the "external" keyword over "interop".

@DBDuncan DBDuncan requested review from a team as code owners July 4, 2024 12:13
@DBDuncan DBDuncan requested a review from GeorgeWeb July 4, 2024 12:13
@DBDuncan DBDuncan mentioned this pull request Jul 4, 2024
Merged
@DBDuncan
Copy link
Contributor Author

DBDuncan commented Jul 4, 2024

Corresponding DPC++ PR: intel/llvm#14444

@github-actions github-actions bot added loader Loader related feature/bug common Changes or additions to common utilities images UR images specification Changes or additions to the specification experimental Experimental feature additions/changes/specification level-zero L0 adapter specific issues cuda CUDA adapter specific issues hip HIP adapter specific issues native-cpu Native CPU adapter specific issues labels Jul 4, 2024
@DBDuncan DBDuncan force-pushed the sean/rename-interop-to-external branch from 2875c7e to b7c174f Compare July 8, 2024 12:37
@DBDuncan
Copy link
Contributor Author

DBDuncan commented Jul 8, 2024

Friendly ping @oneapi-src/unified-runtime-native-cpu-write @oneapi-src/unified-runtime-cuda-write @oneapi-src/unified-runtime-level-zero-write @GeorgeWeb

@DBDuncan DBDuncan force-pushed the sean/rename-interop-to-external branch 2 times, most recently from 4c2cee1 to 4095b4a Compare July 9, 2024 15:42
pbalcer
pbalcer approved these changes Jul 12, 2024
View reviewed changes
Copy link
Contributor

@pbalcer pbalcer left a comment

Choose a reason for hiding this comment

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

lgtm, just a minor nit.

scripts/core/registry.yml Show resolved Hide resolved
@cppchedy cppchedy force-pushed the sean/rename-interop-to-external branch from 4095b4a to d1a7c66 Compare July 12, 2024 16:04
@DBDuncan DBDuncan force-pushed the sean/rename-interop-to-external branch from d1a7c66 to c799e69 Compare July 15, 2024 15:49
@DBDuncan
Copy link
Contributor Author

Friendly ping @oneapi-src/unified-runtime-native-cpu-write @oneapi-src/unified-runtime-cuda-write @GeorgeWeb

PietroGhg
PietroGhg approved these changes Jul 16, 2024
View reviewed changes
Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Native CPU lgtm, thank you

@DBDuncan DBDuncan force-pushed the sean/rename-interop-to-external branch from c799e69 to e895655 Compare July 16, 2024 16:28
GeorgeWeb
GeorgeWeb approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@GeorgeWeb GeorgeWeb left a comment

Choose a reason for hiding this comment

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

The CUDA adapter changes look good to me.
Only aside is @npmiller's comment about the function symbols not being in the ur_interface_loader for HIP. It is not directly related to this PR but it will be good to know if his concern is actually the case, so we may follow-up.

@DBDuncan
Copy link
Contributor Author

The CUDA adapter changes look good to me. Only aside is @npmiller's comment about the function symbols not being in the ur_interface_loader for HIP. It is not directly related to this PR but it will be good to know if his concern is actually the case, so we may follow-up.

HIP is currently not supported at all in bindless images. Though the function symbols should exist. A new task will be created to fix this later.

@GeorgeWeb
Copy link
Contributor

GeorgeWeb commented Jul 17, 2024 •
edited
Loading

The CUDA adapter changes look good to me. Only aside is @npmiller's comment about the function symbols not being in the ur_interface_loader for HIP. It is not directly related to this PR but it will be good to know if his concern is actually the case, so we may follow-up.

HIP is currently not supported at all in bindless images. Though the function symbols should exist. A new task will be created to fix this later.

Yeah that makes sense. Thanks for clarifying. For the purposes of this PR, Cuda and HIP changes look good then.

@DBDuncan
Copy link
Contributor Author

@npmiller Created an internal ticket to add the missing functions to HIP's 'ur_interface_loader.cpp`.

@DBDuncan DBDuncan force-pushed the sean/rename-interop-to-external branch 2 times, most recently from 32c7b49 to 8270ca3 Compare July 18, 2024 16:19
@DBDuncan DBDuncan added the ready to merge Added to PR's which are ready to merge label Jul 19, 2024
@DBDuncan DBDuncan force-pushed the sean/rename-interop-to-external branch 3 times, most recently from 8bf7f48 to 9d6b7d8 Compare July 22, 2024 14:57
@alycm alycm added the v0.10.x Include in the v0.10.x release label Jul 24, 2024
@kbenzie
Copy link
Contributor

kbenzie commented Jul 24, 2024 •
edited
Loading

this has multiple conflicts resolved

@DBDuncan DBDuncan force-pushed the sean/rename-interop-to-external branch from 9d6b7d8 to 9e87503 Compare July 24, 2024 11:38
@Seanst98 @DBDuncan
[Bindless][Exp] Rename interop related structs/funcs with "external"
572c22f 
Rename interop related structs, functions, and enums with the
"external" keyword over "interop".
@DBDuncan DBDuncan force-pushed the sean/rename-interop-to-external branch from 9e87503 to 572c22f Compare July 24, 2024 15:58
@omarahmed1111 omarahmed1111 merged commit bc1a28e into oneapi-src:main Jul 29, 2024
55 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@npmiller npmiller npmiller left review comments

@kbenzie kbenzie kbenzie left review comments

@PietroGhg PietroGhg PietroGhg approved these changes

@aarongreig aarongreig aarongreig approved these changes

@isaacault isaacault isaacault approved these changes

@pbalcer pbalcer pbalcer approved these changes

@GeorgeWeb GeorgeWeb GeorgeWeb approved these changes

Assignees
No one assigned
Labels
common Changes or additions to common utilities cuda CUDA adapter specific issues experimental Experimental feature additions/changes/specification hip HIP adapter specific issues images UR images level-zero L0 adapter specific issues loader Loader related feature/bug native-cpu Native CPU adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification v0.10.x Include in the v0.10.x release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.