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

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

Merged
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

@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
@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
- 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
@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
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
Development

Successfully merging this pull request may close these issues.

3 participants