Skip to content

[UR][CUDA][HIP] Cleanup adapter code #18370

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

Merged
merged 5 commits into from
May 13, 2025
Merged

Conversation

npmiller
Copy link
Contributor

@npmiller npmiller commented May 8, 2025

  • Inline createProgram function in CUDA adapter, and catch ur result exceptions in both adapters.
  • Fix pattern of unnecessarily storing error code in a variable instead of just returning it.
  • Fix incorrect setErrorMessage uses, the adapter specific code signals that there is a specific error code and message in get last error, it doesn't make sense to use it as the code in the last error.
  • Remove now unrelated comment on now removed primary context extension.
  • Don't use unique_ptr when delete is enough.

@npmiller npmiller requested review from a team as code owners May 8, 2025 17:27
@npmiller npmiller requested a review from jchlanda May 8, 2025 17:27
@npmiller npmiller temporarily deployed to WindowsCILock May 8, 2025 17:27 — with GitHub Actions Inactive
@npmiller npmiller temporarily deployed to WindowsCILock May 8, 2025 18:00 — with GitHub Actions Inactive
@npmiller npmiller temporarily deployed to WindowsCILock May 8, 2025 18:00 — with GitHub Actions Inactive
@npmiller npmiller force-pushed the cuda-hip-cleanups branch from 9774f55 to 954e22e Compare May 9, 2025 14:58
@npmiller npmiller temporarily deployed to WindowsCILock May 9, 2025 14:58 — with GitHub Actions Inactive
@npmiller npmiller temporarily deployed to WindowsCILock May 9, 2025 17:56 — with GitHub Actions Inactive
@npmiller npmiller temporarily deployed to WindowsCILock May 9, 2025 17:56 — with GitHub Actions Inactive
if (Ret != hipSuccess && Ret != hipErrorNotFound)
UR_CHECK_ERROR(Ret);
return mapErrorUR(Ret);

if (Ret == hipErrorNotFound) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nit, but if we check for hipErrorNotFound first, we wouldn't need to re-check for it in the second if. Maybe tying them into if/else if would be nicer?

@jchlanda
Copy link
Contributor

  • that that

s/that that/that

In the commit message :)

npmiller added 2 commits May 12, 2025 08:23
- Inline createProgram function in CUDA adapter, and catch ur result
  exceptions in both adapters.
- Fix pattern of unnecessarily storing error code in a variable instead
  of just returning it.
- Fix incorrect setErrorMessage uses, the adapter specific code signals
  that that there is a specific error code and message in get last
  error, it doesn't make sense to use it as the code in the last error.
- Remove now unrelated comment on now removed primary context extension.
- Don't use `unique_ptr` when `delete` is enough.
Copy link
Contributor

@przemektmalon przemektmalon left a comment

Choose a reason for hiding this comment

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

Bindless Images LGTM.

@npmiller npmiller temporarily deployed to WindowsCILock May 13, 2025 09:24 — with GitHub Actions Inactive
@npmiller
Copy link
Contributor Author

@intel/llvm-gatekeepers this is ready to merge, failure unrelated and tracked here: #18416

@kbenzie kbenzie merged commit 79f28f6 into intel:sycl May 13, 2025
32 of 33 checks passed
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.

4 participants