[UR][SYCL] Implement USM prefetch from device to host in SYCL runtime and UR #19437
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
|
|
||
| #include <sycl/detail/common.hpp> | ||
| #include <sycl/event.hpp> | ||
| #include <sycl/ext/oneapi/experimental/enqueue_types.hpp> | ||
| #include <sycl/ext/oneapi/experimental/graph.hpp> | ||
| #include <sycl/ext/oneapi/properties/properties.hpp> | ||
| #include <sycl/handler.hpp> | ||
|
|
@@ -369,15 +370,25 @@ void fill(sycl::queue Q, T *Ptr, const T &Pattern, size_t Count, | |
| CodeLoc); | ||
| } | ||
|
|
||
| inline void prefetch(handler &CGH, void *Ptr, size_t NumBytes, | ||
| prefetch_type Type = prefetch_type::device) { | ||
| #ifdef __INTEL_PREVIEW_BREAKING_CHANGES | ||
|
There was a problem hiding this comment. This is an experimental API, so is there a reason we won't change the behavior? Does this path drastically change existing behavior? There was a problem hiding this comment. Looking at this quickly because @steffenlarsen mentioned me. I think the However, I don't see the reason for adding or for that matter, just create a new overload of There's no need to preserve ABI in the case when the application uses a new feature that didn't exist in the previous compiler release. Therefore, if the application passes There was a problem hiding this comment. This is a lot simpler than how I had it, I went ahead with this approach. Works fine with older versions of libsycl.so as well. There was a problem hiding this comment.
@gmlueck - Do you mean compile with a new compiler, then run with an old libsycl? That is not something we support. We only support old builds running with new libsycl. Unless I misunderstood, the question then is whether There was a problem hiding this comment.
Oh, good point. In that case, can't we simplify it further and just do this: (Note this calls the new We'd still keep the overload with 2 parameters for backward compatibility with applications compiled with the old compiler. We'd need to make sure the 2-parameter overload behaves the same as it did in the old compiler, but isn't that the case? There was a problem hiding this comment.
So that's the part I'm unsure about. If it does, then I don't see any reason we can't switch to it. If it does differ, I wonder how drastically it differs. There was a problem hiding this comment. Since support for older libsycl's doesn't matter, I've gone ahead with the change Greg proposed.
The behavior in the normal code path for prefetch (in the runtime*) should be the "same", although it's worth noting I have also modified the UR here. Despite that, I'm hoping tests can at least show that the runtime implementation behave scorrectly: Unit testing to make sure the correct parameters are used when invoking the UR from runtime:
E2E checks to make sure functions behave as expected:
If we know that prefetch is invoking the correct UR call, and there are no errors when we make said UR call, would it be sufficient to say that at least the runtime implementation has no faults in it? As for the correctness of the UR implementation, I assume that's up to the UR reviewers... There was a problem hiding this comment. Based on the UR changes, I agree that there shouldn't be any difference in behavior for the |
||
| CGH.ext_oneapi_prefetch_exp(Ptr, NumBytes, Type); | ||
| #else | ||
| if (Type == prefetch_type::device) { | ||
| CGH.prefetch(Ptr, NumBytes); | ||
| } else { | ||
| CGH.ext_oneapi_prefetch_d2h(Ptr, NumBytes); | ||
| } | ||
| #endif | ||
| } | ||
|
|
||
| inline void prefetch(queue Q, void *Ptr, size_t NumBytes, | ||
| prefetch_type Type = prefetch_type::device, | ||
| const sycl::detail::code_location &CodeLoc = | ||
| sycl::detail::code_location::current()) { | ||
| submit( | ||
| std::move(Q), [&](handler &CGH) { prefetch(CGH, Ptr, NumBytes, Type); }, | ||
| CodeLoc); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| //==--------------- enqueue_types.hpp ---- SYCL enqueue types --------------==// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <string> | ||
|
|
||
| namespace sycl { | ||
| inline namespace _V1 { | ||
| namespace ext::oneapi::experimental { | ||
|
|
||
| /// @brief Indicates the destination device for USM data to be prefetched to. | ||
| enum class prefetch_type { device, host }; | ||
|
|
||
| inline std::string prefetchTypeToString(prefetch_type value) { | ||
| switch (value) { | ||
| case sycl::ext::oneapi::experimental::prefetch_type::device: | ||
| return "prefetch_type::device"; | ||
| case sycl::ext::oneapi::experimental::prefetch_type::host: | ||
| return "prefetch_type::host"; | ||
| default: | ||
| return "prefetch_type::unknown"; | ||
| } | ||
| } | ||
|
|
||
| } // namespace ext::oneapi::experimental | ||
| } // namespace _V1 | ||
| } // namespace sycl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the name here should be fine. It's not part of the ABI by its name, nor does the user have access to it.