Skip to content

[CMDBUF] Fix incorrect handling of shared local mem args in CUDA/HIP #2264

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
Merged
merged 1 commit into from
Nov 6, 2024
Merged

[CMDBUF] Fix incorrect handling of shared local mem args in CUDA/HIP #2264

merged 1 commit into from
Nov 6, 2024

Conversation

@Bensuo
Copy link
Contributor

@Bensuo Bensuo commented Oct 29, 2024

  • Fix handling of local mem args in CUDA/HIP for command buffer update
  • Add conformance tests which check updating local memory args and work size

@Bensuo Bensuo requested review from a team as code owners October 29, 2024 17:11
@github-actions github-actions bot added conformance Conformance test suite issues. cuda CUDA adapter specific issues hip HIP adapter specific issues command-buffer Command Buffer feature addition/changes/specification labels Oct 29, 2024
@Bensuo
Copy link
Contributor Author

Bensuo commented Oct 29, 2024

DPC++ PR: intel/llvm#15920

EwanC
EwanC reviewed Oct 30, 2024
View reviewed changes
@Bensuo Bensuo force-pushed the ben/cmdbuf-local-arg-fix branch 2 times, most recently from b9eb5da to 31c441e Compare October 30, 2024 17:39
@github-actions github-actions bot added specification Changes or additions to the specification experimental Experimental feature additions/changes/specification labels Oct 30, 2024
@Bensuo Bensuo force-pushed the ben/cmdbuf-local-arg-fix branch from 31c441e to 5e9aa0f Compare October 30, 2024 17:43
@EwanC
Copy link
Contributor

EwanC commented Oct 31, 2024

Code change look fine to me, but there's some HIP command-buffer CTS tests failing in CI which might not be spurious

@Bensuo Bensuo force-pushed the ben/cmdbuf-local-arg-fix branch 3 times, most recently from 2edb77c to 986de59 Compare October 31, 2024 19:15
@Bensuo
[CMDBUF] Fix incorrect handling of shared local mem args in CUDA/HIP
b7d78ba 
- Fix handling of local mem args in CUDA/HIP
- Add conformance tests which check updating local memory args and work size
- Change argSize in ur_exp_command_buffer_update_value_arg_desc_t to be size_t in line with urKernelSetArg
@Bensuo Bensuo force-pushed the ben/cmdbuf-local-arg-fix branch from 986de59 to b7d78ba Compare October 31, 2024 19:23
@EwanC
Copy link
Contributor

EwanC commented Nov 1, 2024

@oneapi-src/unified-runtime-maintain This is ready for review, there is a CUDA E2E CI fail but that's not reflected in the CI for the matching DPC++ PR which is green and also seems to be in other open PRs like #2263

Failed Tests (14):
  SYCL :: Basic/kernel_info.cpp
  SYCL :: Basic/kernel_info_attr.cpp
  SYCL :: Basic/launch_queries/max_num_work_groups.cpp
  SYCL :: DeviceCodeSplit/split-per-kernel.cpp
  SYCL :: DeviceCodeSplit/split-per-source-main.cpp
  SYCL :: Graph/Explicit/spec_constants_kernel_bundle_api.cpp
  SYCL :: Graph/RecordReplay/spec_constants_kernel_bundle_api.cpp
  SYCL :: GroupAlgorithm/back_to_back_collectives.cpp
  SYCL :: GroupAlgorithm/root_group.cpp
  SYCL :: KernelAndProgram/kernel-bundle-find-run.cpp
  SYCL :: SpecConstants/2020/kernel-bundle-api.cpp
  SYCL :: SubGroup/info.cpp
  SYCL :: USM/P2P/p2p_access.cpp
  SYCL :: syclcompat/util/max_active_work_groups_per_cu.cpp

@callumfare callumfare added the ready to merge Added to PR's which are ready to merge label Nov 4, 2024
@callumfare callumfare merged commit 5955bad into oneapi-src:main Nov 6, 2024
76 of 77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@EwanC EwanC EwanC approved these changes

@callumfare callumfare callumfare approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

command-buffer Command Buffer feature addition/changes/specification conformance Conformance test suite issues. cuda CUDA adapter specific issues experimental Experimental feature additions/changes/specification hip HIP adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Bensuo @EwanC @callumfare