Skip to content

[SYCL] Fix the check for read-only host pointer during memobj creation #2697

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

Merged
merged 2 commits into from
Nov 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions sycl/source/detail/buffer_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,26 @@ namespace sycl {
namespace detail {
void *buffer_impl::allocateMem(ContextImplPtr Context, bool InitFromUserData,
void *HostPtr, RT::PiEvent &OutEventToWait) {
// The host pointer for the allocation can be provided in 2 ways:
// 1. Initialize the allocation from user data. Check if the user pointer is
// read-only.
// 2. Use a HostPtr allocated by the runtime. Assume any such pointer to be
// read-write.
bool HostPtrReadOnly = false;
if (InitFromUserData) {
assert(!HostPtr && "Cannot init from user data and reuse host ptr provided "
"simultaneously");
HostPtr = BaseT::getUserPtr();
HostPtrReadOnly = BaseT::MHostPtrReadOnly;
}

assert(!(InitFromUserData && HostPtr) &&
"Cannot init from user data and reuse host ptr provided "
"simultaneously");

void *UserPtr = InitFromUserData ? BaseT::getUserPtr() : HostPtr;

assert(!(nullptr == UserPtr && BaseT::useHostPtr() && Context->is_host()) &&
"Internal error. Allocating memory on the host "
"while having use_host_ptr property");
assert(!(nullptr == HostPtr && BaseT::useHostPtr() && Context->is_host()) &&
"Internal error. Allocating memory on the host "
"while having use_host_ptr property");

return MemoryManager::allocateMemBuffer(
std::move(Context), this, UserPtr, BaseT::MHostPtrReadOnly,
BaseT::getSize(), BaseT::MInteropEvent, BaseT::MInteropContext, MProps,
OutEventToWait);
std::move(Context), this, HostPtr, HostPtrReadOnly, BaseT::getSize(),
BaseT::MInteropEvent, BaseT::MInteropContext, MProps, OutEventToWait);
}
} // namespace detail
} // namespace sycl
Expand Down
28 changes: 17 additions & 11 deletions sycl/source/detail/image_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,25 +307,31 @@ template <int Dimensions>
void *image_impl<Dimensions>::allocateMem(ContextImplPtr Context,
bool InitFromUserData, void *HostPtr,
RT::PiEvent &OutEventToWait) {
// The host pointer for the allocation can be provided in 2 ways:
// 1. Initialize the allocation from user data. Check if the user pointer is
// read-only.
// 2. Use a HostPtr allocated by the runtime. Assume any such pointer to be
// read-write.
bool HostPtrReadOnly = false;
if (InitFromUserData) {
assert(!HostPtr && "Cannot init from user data and reuse host ptr provided "
"simultaneously");
HostPtr = BaseT::getUserPtr();
HostPtrReadOnly = BaseT::MHostPtrReadOnly;
}

assert(!(InitFromUserData && HostPtr) &&
"Cannot init from user data and reuse host ptr provided "
"simultaneously");

void *UserPtr = InitFromUserData ? BaseT::getUserPtr() : HostPtr;

RT::PiMemImageDesc Desc = getImageDesc(UserPtr != nullptr);
assert(checkImageDesc(Desc, Context, UserPtr) &&
RT::PiMemImageDesc Desc = getImageDesc(HostPtr != nullptr);
assert(checkImageDesc(Desc, Context, HostPtr) &&
"The check an image desc failed.");

RT::PiMemImageFormat Format = getImageFormat();
assert(checkImageFormat(Format, Context) &&
"The check an image format failed.");

return MemoryManager::allocateMemImage(
std::move(Context), this, UserPtr, BaseT::MHostPtrReadOnly,
BaseT::getSize(), Desc, Format, BaseT::MInteropEvent,
BaseT::MInteropContext, MProps, OutEventToWait);
std::move(Context), this, HostPtr, HostPtrReadOnly, BaseT::getSize(),
Desc, Format, BaseT::MInteropEvent, BaseT::MInteropContext, MProps,
OutEventToWait);
}

template <int Dimensions>
Expand Down
28 changes: 28 additions & 0 deletions sycl/test/basic_tests/buffer/native_buffer_creation_flags.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out
// RUN: %CPU_RUN_PLACEHOLDER SYCL_PI_TRACE=-1 %t.out 2>&1 %CPU_CHECK_PLACEHOLDER

#include <CL/sycl.hpp>

class Foo;
using namespace cl::sycl;
int main() {
const int BufVal = 42;
buffer<int, 1> Buf{&BufVal, range<1>(1)};
queue Q;

{
// This should trigger memory allocation on host since the pointer passed by
// the user is read-only.
auto BufAcc = Buf.get_access<access::mode::write>();
}

Q.submit([&](handler &Cgh) {
// Now that we have a read-write host allocation, check that the native
// buffer is created with the PI_MEM_FLAGS_HOST_PTR_USE flag.
// CHECK: piMemBufferCreate
// CHECK-NEXT: {{.*}} : {{.*}}
// CHECK-NEXT: {{.*}} : 9
auto BufAcc = Buf.get_access<access::mode::read>(Cgh);
Cgh.single_task<Foo>([=]() { int A = BufAcc[0]; });
});
}