Skip to content

Commit

Permalink
[SYCL] Fix initialization of interoperability memory objects (#4205)
Browse files Browse the repository at this point in the history
Fix an issue where the native memory object passed to interoperability
memory object constructor was ignored on devices without host unified
memory.
  • Loading branch information
sergey-semenov authored Aug 2, 2021
1 parent 6bfa84e commit da19678
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 3 deletions.
2 changes: 2 additions & 0 deletions sycl/include/CL/sycl/detail/sycl_mem_obj_t.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ class __SYCL_EXPORT SYCLMemObjT : public SYCLMemObjI {

bool hasUserDataPtr() const { return MUserPtr != nullptr; };

bool isInterop() const;

protected:
// An allocateMem helper that determines which host ptr to use
void determineHostPtr(const ContextImplPtr &Context, bool InitFromUserData,
Expand Down
6 changes: 3 additions & 3 deletions sycl/source/detail/scheduler/graph_builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -677,12 +677,12 @@ AllocaCommandBase *Scheduler::GraphBuilder::getOrCreateAllocaForReq(
// unnecessary copy on devices with unified host memory support.
const bool HostUnifiedMemory =
checkHostUnifiedMemory(Queue->getContextImplPtr());
const bool InitFromUserData =
Record->MAllocaCommands.empty() && HostUnifiedMemory;
AllocaCommandBase *LinkedAllocaCmd = nullptr;
// TODO casting is required here to get the necessary information
// without breaking ABI, replace with the next major version.
auto *MemObj = static_cast<SYCLMemObjT *>(Req->MSYCLMemObj);
const bool InitFromUserData = Record->MAllocaCommands.empty() &&
(HostUnifiedMemory || MemObj->isInterop());
AllocaCommandBase *LinkedAllocaCmd = nullptr;

// For the first allocation on a device without host unified memory we
// might need to also create a host alloca right away in order to perform
Expand Down
2 changes: 2 additions & 0 deletions sycl/source/detail/sycl_mem_obj_t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ size_t SYCLMemObjT::getBufSizeForContext(const ContextImplPtr &Context,
return BufSize;
}

bool SYCLMemObjT::isInterop() const { return MOpenCLInterop; }

void SYCLMemObjT::determineHostPtr(const ContextImplPtr &Context,
bool InitFromUserData, void *&HostPtr,
bool &HostPtrReadOnly) {
Expand Down
1 change: 1 addition & 0 deletions sycl/test/abi/sycl_symbols_linux.dump
Original file line number Diff line number Diff line change
Expand Up @@ -4041,6 +4041,7 @@ _ZNK2cl4sycl6detail10image_implILi3EE7getTypeEv
_ZNK2cl4sycl6detail10image_implILi3EE9get_countEv
_ZNK2cl4sycl6detail10image_implILi3EE9get_rangeEv
_ZNK2cl4sycl6detail11SYCLMemObjT9getPluginEv
_ZNK2cl4sycl6detail11SYCLMemObjT9isInteropEv
_ZNK2cl4sycl6detail11stream_impl22get_max_statement_sizeEv
_ZNK2cl4sycl6detail11stream_impl8get_sizeEv
_ZNK2cl4sycl6detail12sampler_impl18get_filtering_modeEv
Expand Down
37 changes: 37 additions & 0 deletions sycl/unittests/scheduler/NoHostUnifiedMemory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,19 @@ static pi_result redefinedEnqueueMemBufferWriteRect(
return PI_SUCCESS;
}

static pi_result redefinedMemRetain(pi_mem mem) { return PI_SUCCESS; }
static pi_result redefinedMemRelease(pi_mem mem) { return PI_SUCCESS; }

static pi_context InteropPiContext = nullptr;
static pi_result redefinedMemGetInfo(pi_mem mem, cl_mem_info param_name,
size_t param_value_size, void *param_value,
size_t *param_value_size_ret) {
EXPECT_EQ(param_name, static_cast<cl_mem_info>(CL_MEM_CONTEXT));
auto *Result = reinterpret_cast<pi_context *>(param_value);
*Result = InteropPiContext;
return PI_SUCCESS;
}

TEST_F(SchedulerTest, NoHostUnifiedMemory) {
platform Plt{default_selector()};
if (Plt.is_host()) {
Expand All @@ -75,7 +86,9 @@ TEST_F(SchedulerTest, NoHostUnifiedMemory) {
redefinedEnqueueMemBufferReadRect);
Mock.redefine<detail::PiApiKind::piEnqueueMemBufferWriteRect>(
redefinedEnqueueMemBufferWriteRect);
Mock.redefine<detail::PiApiKind::piMemRetain>(redefinedMemRetain);
Mock.redefine<detail::PiApiKind::piMemRelease>(redefinedMemRelease);
Mock.redefine<detail::PiApiKind::piMemGetInfo>(redefinedMemGetInfo);
cl::sycl::detail::QueueImplPtr QImpl = detail::getSyclObjImpl(Q);

device HostDevice;
Expand Down Expand Up @@ -185,4 +198,28 @@ TEST_F(SchedulerTest, NoHostUnifiedMemory) {
// The current context for the record should still be modified.
EXPECT_EQ(Record->MCurContext, DefaultHostQueue->getContextImplPtr());
}
// Check that interoperability memory objects are initialized.
{
cl_mem MockInteropBuffer = reinterpret_cast<cl_mem>(1);
context InteropContext = Q.get_context();
InteropPiContext = detail::getSyclObjImpl(InteropContext)->getHandleRef();
std::shared_ptr<detail::buffer_impl> BufI = std::make_shared<
detail::buffer_impl>(
MockInteropBuffer, Q.get_context(), /*BufSize*/ 8,
make_unique_ptr<detail::SYCLMemObjAllocatorHolder<buffer_allocator>>(),
event());

detail::Requirement Req = getMockRequirement();
Req.MSYCLMemObj = BufI.get();
std::vector<detail::Command *> AuxCmds;
detail::MemObjRecord *Record =
MS.getOrInsertMemObjRecord(QImpl, &Req, AuxCmds);
detail::AllocaCommandBase *InteropAlloca =
MS.getOrCreateAllocaForReq(Record, &Req, QImpl, AuxCmds);
detail::EnqueueResultT Res;
MockScheduler::enqueueCommand(InteropAlloca, Res, detail::BLOCKING);

EXPECT_EQ(Record->MAllocaCommands.size(), 1U);
EXPECT_EQ(InteropAlloca->MMemAllocation, MockInteropBuffer);
}
}

0 comments on commit da19678

Please sign in to comment.