[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
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