[UR] Add support for bidirectional USM prefetching #2312
Conversation
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
| if (Flags == UR_USM_MIGRATION_FLAG_DEVICE_TO_HOST) { | ||
| logger::warning("USM migration from device to host is not currently " | ||
| "supported by level zero."); | ||
| return UR_RESULT_SUCCESS; | ||
| } |
There was a problem hiding this comment.
This early return is a problem for out-of-order command-lists in that it doesn't respect the command dependencies or signal dependent commands when finished.
In the else branch below think we just want to opt-out of the actual zeCommandListAppendMemoryPrefetch but keep the zeCommandListAppendWaitOnEvents and zeCommandListAppendSignalEvent commnads.
There was a problem hiding this comment.
Hey thanks for reviewing! Admittedly I'm not the most familiar with how things work in L0; would I also need to do this for source/adapters/level_zero/memory.cpp:1243?
There was a problem hiding this comment.
Yes I think that the same issue with an early return, the OutEvent output parameter won't be set and it's not respecting EventWaitList dependencies
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
| {{NONDETERMINISTIC}} | ||
| urEnqueueKernelLaunchTest.InvalidKernelArgs/* | ||
| urEnqueueKernelLaunchKernelWgSizeTest.NonMatchingLocalSize/* | ||
| urEnqueueKernelLaunchKernelSubGroupTest.Success/* | ||
| urEnqueueMemBufferMapTestWithWriteFlagParam.SuccessWrite/*__UR_MAP_FLAG_WRITE_INVALIDATE_REGION | ||
| urEnqueueUSMAdviseWithParamTest.Success/*__UR_USM_ADVICE_FLAG_DEFAULT | ||
| urEnqueueUSMAdviseTest.MultipleParamsSuccess/* | ||
| urEnqueueUSMPrefetchWithParamTest.Success/*__UR_USM_MIGRATION_FLAG_DEFAULT | ||
| urEnqueueUSMPrefetchWithParamTest.CheckWaitEvent/*__UR_USM_MIGRATION_FLAG_DEFAULT | ||
| urEnqueueUSMPrefetchWithParamTest.Success/*__UR_USM_MIGRATION_FLAG_HOST_TO_DEVICE | ||
| urEnqueueUSMPrefetchWithParamTest.Success/*__UR_USM_MIGRATION_FLAG_DEVICE_TO_HOST | ||
| urEnqueueUSMPrefetchWithParamTest.CheckWaitEvent/*__UR_USM_MIGRATION_FLAG_HOST_TO_DEVICE | ||
| urEnqueueUSMPrefetchWithParamTest.CheckWaitEvent/*__UR_USM_MIGRATION_FLAG_DEVICE_TO_HOST | ||
| urEnqueueTimestampRecordingExpTest.Success/* | ||
| urEnqueueTimestampRecordingExpTest.SuccessBlocking/* | ||
| urEnqueueTimestampRecordingExpTest.SuccessBlocking/* |
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
github-actions
bot
added
conformance
| cmd_buf_handle, device_ptrs[0], allocation_size, | ||
| UR_USM_MIGRATION_FLAG_HOST_TO_DEVICE, 0, nullptr, 0, nullptr, nullptr, | ||
| nullptr, nullptr)); | ||
| ASSERT_SUCCESS(urCommandBufferAppendUSMPrefetchExp( |
There was a problem hiding this comment.
My understanding is that changes include calling prefetch here twice for host and device in both directions. Is that consistent across all tests?
There was a problem hiding this comment.
As far as I'm aware I'm checking both directions for only tests checking functionality; e.g. I am not checking both directions if the test is checking how the function handles invalid input. Let me know if you'd like the tests changed to always use both directions regardless.
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
…into ianayl/2way-prefetch
Unified Runtime -> intel/llvm Repo Move NoticeInformationThe source code of Unified Runtime has been moved to intel/llvm under the unified-runtime top-level directory, The code will be mirrored to oneapi-src/unified-runtime and the specification will continue to be hosted at oneapi-src.github.io/unified-runtime. The contribution guide has been updated with new instructions for contributing to Unified Runtime. PR MigrationAll open PRs including this one will be labelled auto-close and shall be automatically closed after 30 days. Should you wish to continue with your PR you will need to migrate it to intel/llvm. This is an automated comment. |
Unified Runtime -> intel/llvm Repo Move NoticeFollowing on from the previous notice, we have now enabled workflows to automatically label and close PRs because the Unified Runtime source code has moved to intel/llvm. This PR has now been marked with the Please review the previous notice for more information, including assistance with migrating your PR to intel/llvm. Should there be a reason for this PR to remain open, manually remove the This is an automated comment. |
Automatic PR Closure NoticeInformationThis PR has been closed automatically. It was marked with the All Unified Runtime development should be done in intel/llvm, details can be found in the updated contribution guide. Next StepsShould you wish to re-open this PR it must be moved to intel/llvm. We have provided a script to help automate this process, otherwise no actions are required. This is an automated comment. |
This PR implements the previously unimplemented
ur_usm_migration_flags_tenum, and adds the logic for prefetching for from the device to host for each implemented adapter.Note that level zero currently does not support this behavior: it remains unimplemented for now until level zero implements a way to prefetch from device to host.
Corresponding intel/llvm PR for testing: intel/llvm#16047