Skip to content

[UR][CUDA][HIP] Fix Set Arg Local #10710

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 1 commit into from
Aug 25, 2023

Conversation

veselypeta
Copy link
Contributor

@veselypeta veselypeta commented Aug 7, 2023

In the CUDA/HIP adapters urKernelSetArgValue was being used to implement both urKernelSetArgValue & urKernelSetArgLocal. However, if the validation layer is enabled in UR then the path to set local arg is never taken since it includes a check that pArgValue is not null.

This PR:

  • Implements urKernelSetArgLocal for CUDA/HIP adapters
  • Changes pi2ur to call urKernelSetArgLocal when arg_value is nullptr
  • Implements urKernelSetArgLocal for L0 adapter - this just calls back into urKernelSetArgValue.

@veselypeta veselypeta temporarily deployed to aws August 7, 2023 09:20 — with GitHub Actions Inactive
@veselypeta veselypeta force-pushed the petr/fix-cuda-set-arg-local branch from 1a1721d to 291dcb3 Compare August 7, 2023 09:52
@veselypeta veselypeta temporarily deployed to aws August 7, 2023 10:02 — with GitHub Actions Inactive
@veselypeta veselypeta force-pushed the petr/fix-cuda-set-arg-local branch from 291dcb3 to 2529422 Compare August 7, 2023 10:27
@veselypeta veselypeta temporarily deployed to aws August 7, 2023 10:37 — with GitHub Actions Inactive
@veselypeta veselypeta temporarily deployed to aws August 7, 2023 11:20 — with GitHub Actions Inactive
@veselypeta veselypeta force-pushed the petr/fix-cuda-set-arg-local branch from 2529422 to bd75f00 Compare August 8, 2023 09:38
@veselypeta veselypeta marked this pull request as ready for review August 8, 2023 14:10
@veselypeta veselypeta requested review from a team as code owners August 8, 2023 14:10
@veselypeta veselypeta requested a review from jchlanda August 8, 2023 14:10
@veselypeta veselypeta temporarily deployed to aws August 9, 2023 16:33 — with GitHub Actions Inactive
@veselypeta veselypeta closed this Aug 10, 2023
@veselypeta veselypeta reopened this Aug 10, 2023
@jchlanda
Copy link
Contributor

CUDA/HIP 👍

@hdelan
Copy link
Contributor

hdelan commented Aug 18, 2023

Ping @intel/dpcpp-l0-pi-reviewers can we get a review on this?

@veselypeta veselypeta force-pushed the petr/fix-cuda-set-arg-local branch from bd75f00 to a7653f3 Compare August 25, 2023 10:39
@veselypeta
Copy link
Contributor Author

@intel/llvm-gatekeepers can this be merged?

@steffenlarsen steffenlarsen merged commit a3595f1 into intel:sycl Aug 25, 2023
@veselypeta veselypeta deleted the petr/fix-cuda-set-arg-local branch August 29, 2023 13:16
veselypeta added a commit to veselypeta/llvm that referenced this pull request Sep 21, 2023
In the CUDA/HIP adapters `urKernelSetArgValue` was being used to
implement both `urKernelSetArgValue` & `urKernelSetArgLocal`. However,
if the validation layer is enabled in UR then the path to set local arg
is never taken since it includes a check that `pArgValue` is not null.

This PR:
 * Implements `urKernelSetArgLocal` for CUDA/HIP adapters
* Changes `pi2ur` to call `urKernelSetArgLocal` when `arg_value` is
`nullptr`
* Implements `urKernelSetArgLocal` for L0 adapter - this just calls back
into `urKernelSetArgValue`.
fabiomestre pushed a commit to fabiomestre/llvm that referenced this pull request Sep 26, 2023
In the CUDA/HIP adapters `urKernelSetArgValue` was being used to
implement both `urKernelSetArgValue` & `urKernelSetArgLocal`. However,
if the validation layer is enabled in UR then the path to set local arg
is never taken since it includes a check that `pArgValue` is not null.

This PR:
 * Implements `urKernelSetArgLocal` for CUDA/HIP adapters
* Changes `pi2ur` to call `urKernelSetArgLocal` when `arg_value` is
`nullptr`
* Implements `urKernelSetArgLocal` for L0 adapter - this just calls back
into `urKernelSetArgValue`.
veselypeta added a commit to veselypeta/llvm that referenced this pull request Sep 28, 2023
In the CUDA/HIP adapters `urKernelSetArgValue` was being used to
implement both `urKernelSetArgValue` & `urKernelSetArgLocal`. However,
if the validation layer is enabled in UR then the path to set local arg
is never taken since it includes a check that `pArgValue` is not null.

This PR:
 * Implements `urKernelSetArgLocal` for CUDA/HIP adapters
* Changes `pi2ur` to call `urKernelSetArgLocal` when `arg_value` is
`nullptr`
* Implements `urKernelSetArgLocal` for L0 adapter - this just calls back
into `urKernelSetArgValue`.
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.

6 participants