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

[UR][Bindless] Initial implementation of bindless images for HIP #2496

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

GeorgeWeb
Copy link
Contributor

@GeorgeWeb GeorgeWeb commented Dec 20, 2024

This PR implements the entry points in the HIP adapter for the Bindless Images extension.

Some features, while implemented, only partially pass the End-to-end tests for them in DPC++ (the PR that enables the support and testing in DPC++: intel/llvm#16439). This is because the HIP runtime doesn't support, for example, some parameter combinations with some image memory types. The not-fully supported features are marked with TODOs in the device info queries for further work.

Signed-off-by: Georgi Mirazchiyski [email protected]

Co-authored-by: Peter Žužek [email protected]

@github-actions github-actions bot added images UR images hip HIP adapter specific issues labels Dec 20, 2024
@GeorgeWeb GeorgeWeb force-pushed the georgi/bindless-hip branch 3 times, most recently from a89b779 to 0912559 Compare December 20, 2024 16:46
@GeorgeWeb GeorgeWeb marked this pull request as ready for review December 23, 2024 15:17
@GeorgeWeb GeorgeWeb requested review from a team as code owners December 23, 2024 15:17
@GeorgeWeb GeorgeWeb requested a review from ldrumm December 23, 2024 15:17
@GeorgeWeb
Copy link
Contributor Author

GeorgeWeb commented Jan 15, 2025

@oneapi-src/unified-runtime-hip-write , @oneapi-src/unified-runtime-bindless-images-write Heyo, that's a whole lot of changes and additions, so I am just pinging as a reminder in case it was lost in messages over the Winter holidays. There is no rush in reviewing immediately, I just wanted to remind about it. Thank you!

Copy link
Contributor

@frasercrmck frasercrmck left a comment

Choose a reason for hiding this comment

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

I can't say I was able to give the most thorough review with respect to specific HIP APIs or all of the ins and outs of the image properties and logic concerning rows and pitches and so on.

@GeorgeWeb GeorgeWeb force-pushed the georgi/bindless-hip branch 2 times, most recently from 78766a6 to 9996c42 Compare January 27, 2025 17:01
GeorgeWeb and others added 2 commits January 29, 2025 16:48
This PR implements the entry points in the HIP adapter for the Bindless Images extension.

Some features, while implemented, only partially pass the End-to-end tests for them in
DPC++ (the PR that enables the support and testing in DPC++: intel/llvm#16439).
This is because the HIP runtime doesn't support, for example, some parameter combinations
with some image memory types. The not-fully supported features are marked with TODOs in
the device info queries for further work.

Signed-off-by: Georgi Mirazchiyski <[email protected]>

Co-authored-by: Peter Žužek <[email protected]>
@GeorgeWeb GeorgeWeb force-pushed the georgi/bindless-hip branch from 9996c42 to d0c8716 Compare January 29, 2025 16:49
@GeorgeWeb
Copy link
Contributor Author

I can't say I was able to give the most thorough review with respect to specific HIP APIs or all of the ins and outs of the image properties and logic concerning rows and pitches and so on.

@frasercrmck Firstly, apologies for the delay in addressing the feedback. Now this feedback was actually quite thorough and I really appreciate it, so thank you for this!

@GeorgeWeb GeorgeWeb added the ready to merge Added to PR's which are ready to merge label Jan 31, 2025
@kbenzie kbenzie merged commit 0e6adfb into oneapi-src:main Feb 5, 2025
26 checks passed
ldrumm pushed a commit to intel/llvm that referenced this pull request Feb 6, 2025
…on HIP (#16439)

This PR implements the required spirv builtins in the libclc for AMD
targets to support fetching (excluding sampled fetch for now), sampling,
and writing for images and image arrays.

Additionally, End-to-end tests are updated to require the aspects
corresponding to the feature that is being tested from the Bindless
Images extension. This helps avoid having to manually say which backend
adapter is supported or unsupported and instead rely on support based on
aspect/device queries to drive the execution of the tests.

HIP adapter implementation in UR:
oneapi-src/unified-runtime#2496

Signed-off-by: Georgi Mirazchiyski <[email protected]>

---------

Signed-off-by: Georgi Mirazchiyski <[email protected]>
Signed-off-by: JackAKirk <[email protected]>
Co-authored-by: JackAKirk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hip HIP adapter specific issues images UR images ready to merge Added to PR's which are ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants