[SYCL] Fix bug with introp buffer (#2637)

Fixed a problem which happened when a buffer constructed using
interaperablity constructor with sycl::context A is used in a different
context.

Author: romanovvlad

sycl/include/CL/sycl/detail/sycl_mem_obj_i.hpp

@@ -61,6 +61,10 @@ class SYCLMemObjI {
   // Returns size of object in bytes
   virtual size_t getSize() const = 0;
 
+  // Returns the context which is passed if a memory object is created using
+  // interoperability constructor, nullptr otherwise.
+  virtual ContextImplPtr getInteropContext() const = 0;
+
 protected:
   // Pointer to the record that contains the memory commands. This is managed
   // by the scheduler.

sycl/include/CL/sycl/detail/sycl_mem_obj_t.hpp

@@ -287,6 +287,8 @@ class __SYCL_EXPORT SYCLMemObjT : public SYCLMemObjI {
 
   DLL_LOCAL MemObjType getType() const override { return UNDEFINED; }
 
+  ContextImplPtr getInteropContext() const override { return MInteropContext; }
+
 protected:
   // Allocator used for allocation memory on host.
   unique_ptr_class<SYCLMemObjAllocator> MAllocator;

sycl/source/detail/memory_manager.cpp

@@ -125,21 +125,16 @@ void *MemoryManager::allocateInteropMemObject(
     ContextImplPtr TargetContext, void *UserPtr,
     const EventImplPtr &InteropEvent, const ContextImplPtr &InteropContext,
     const sycl::property_list &, RT::PiEvent &OutEventToWait) {
-  // If memory object is created with interop c'tor.
-  // Return cl_mem as is if contexts match.
-  if (TargetContext == InteropContext) {
-    OutEventToWait = InteropEvent->getHandleRef();
-    // Retain the event since it will be released during alloca command
-    // destruction
-    if (nullptr != OutEventToWait) {
-      const detail::plugin &Plugin = InteropEvent->getPlugin();
-      Plugin.call<PiApiKind::piEventRetain>(OutEventToWait);
-    }
-    return UserPtr;
+  // If memory object is created with interop c'tor return cl_mem as is.
+  assert(TargetContext == InteropContext && "Expected matching contexts");
+  OutEventToWait = InteropEvent->getHandleRef();
+  // Retain the event since it will be released during alloca command
+  // destruction
+  if (nullptr != OutEventToWait) {
+    const detail::plugin &Plugin = InteropEvent->getPlugin();
+    Plugin.call<PiApiKind::piEventRetain>(OutEventToWait);
   }
-  // Allocate new cl_mem and initialize from user provided one.
-  assert(false && "Not implemented");
-  return nullptr;
+  return UserPtr;
 }
 
 void *MemoryManager::allocateImageObject(ContextImplPtr TargetContext,

sycl/source/detail/queue_impl.hpp

@@ -142,7 +142,7 @@ class queue_impl {
 
   const plugin &getPlugin() const { return MContext->getPlugin(); }
 
-  ContextImplPtr getContextImplPtr() const { return MContext; }
+  const ContextImplPtr &getContextImplPtr() const { return MContext; }
 
   /// \return an associated SYCL device.
   device get_device() const { return createSyclObjFromImpl<device>(MDevice); }

sycl/source/detail/scheduler/graph_builder.cpp

@@ -176,8 +176,29 @@ Scheduler::GraphBuilder::getOrInsertMemObjRecord(const QueueImplPtr &Queue,
         --(Dependency->MLeafCounter);
       };
 
-  MemObject->MRecord.reset(new MemObjRecord{Queue->getContextImplPtr(),
-                                            LeafLimit, AllocateDependency});
+  const ContextImplPtr &InteropCtxPtr = Req->MSYCLMemObj->getInteropContext();
+  if (InteropCtxPtr) {
+    // The memory object has been constructed using interoperability constructor
+    // which means that there is already an allocation(cl_mem) in some context.
+    // Registering this allocation in the SYCL graph.
+
+    sycl::vector_class<sycl::device> Devices =
+        InteropCtxPtr->get_info<info::context::devices>();
+    assert(Devices.size() != 0);
+    DeviceImplPtr Dev = detail::getSyclObjImpl(Devices[0]);
+
+    // Since all the Scheduler commands require queue but we have only context
+    // here, we need to create a dummy queue bound to the context and one of the
+    // devices from the context.
+    QueueImplPtr InteropQueuePtr{new detail::queue_impl{
+        Dev, InteropCtxPtr, /*AsyncHandler=*/{}, /*PropertyList=*/{}}};
+
+    MemObject->MRecord.reset(
+        new MemObjRecord{InteropCtxPtr, LeafLimit, AllocateDependency});
+    getOrCreateAllocaForReq(MemObject->MRecord.get(), Req, InteropQueuePtr);
+  } else
+    MemObject->MRecord.reset(new MemObjRecord{Queue->getContextImplPtr(),
+                                              LeafLimit, AllocateDependency});
 
   MMemObjs.push_back(MemObject);
   return MemObject->MRecord.get();

sycl/test/basic_tests/buffer/buffer_interop.cpp

@@ -13,13 +13,47 @@
 //
 //===----------------------------------------------------------------------===//
 #include <CL/sycl.hpp>
+
 #include <cassert>
+#include <iostream>
 #include <memory>
 
 using namespace cl::sycl;
 
 int main() {
   bool Failed = false;
+  {
+    sycl::context Ctx;
+    cl_context OCLCtx = Ctx.get();
+
+    cl_int Error = CL_SUCCESS;
+    cl_mem OCLBuf =
+        clCreateBuffer(OCLCtx, CL_MEM_READ_WRITE, sizeof(int), nullptr, &Error);
+    CHECK_OCL_CODE(Error);
+    Error = clReleaseContext(OCLCtx);
+    CHECK_OCL_CODE(Error);
+
+    sycl::buffer<int, 1> Buf{OCLBuf, Ctx};
+
+    sycl::queue Q;
+
+    if (Ctx == Q.get_context()) {
+      std::cerr << "Expected different contexts" << std::endl;
+      Failed = true;
+    }
+
+    Q.submit([&](sycl::handler &CGH) {
+      auto Acc = Buf.get_access<access::mode::write>(CGH);
+      CGH.single_task<class BufferInterop_DifferentContext>(
+          [=]() { Acc[0] = 42; });
+    });
+
+    auto Acc = Buf.get_access<sycl::access::mode::read>();
+    if (Acc[0] != 42) {
+      std::cerr << "Result is incorrect" << std::endl;
+      Failed = true;
+    }
+  }
   {
     constexpr size_t Size = 32;
     int Init[Size] = {5};
@@ -29,9 +63,11 @@ int main() {
 
     queue MyQueue;
 
-    cl_mem OpenCLBuffer = clCreateBuffer(
-        MyQueue.get_context().get(), CL_MEM_READ_WRITE | CL_MEM_COPY_HOST_PTR,
-        Size * sizeof(int), Init, &Error);
+    cl_context OCLCtx = MyQueue.get_context().get();
+
+    cl_mem OpenCLBuffer =
+        clCreateBuffer(OCLCtx, CL_MEM_READ_WRITE | CL_MEM_COPY_HOST_PTR,
+                       Size * sizeof(int), Init, &Error);
     CHECK_OCL_CODE(Error);
     buffer<int, 1> Buffer{OpenCLBuffer, MyQueue.get_context()};
 
@@ -79,6 +115,8 @@ int main() {
         Failed = true;
       }
     }
+    Error = clReleaseContext(OCLCtx);
+    CHECK_OCL_CODE(Error);
   }
   // Check set_final_data
   {
@@ -88,10 +126,11 @@ int main() {
     cl_int Error = CL_SUCCESS;
 
     queue MyQueue;
+    cl_context OCLCtx = MyQueue.get_context().get();
 
-    cl_mem OpenCLBuffer = clCreateBuffer(
-        MyQueue.get_context().get(), CL_MEM_READ_WRITE | CL_MEM_COPY_HOST_PTR,
-        Size * sizeof(int), Init, &Error);
+    cl_mem OpenCLBuffer =
+        clCreateBuffer(OCLCtx, CL_MEM_READ_WRITE | CL_MEM_COPY_HOST_PTR,
+                       Size * sizeof(int), Init, &Error);
     CHECK_OCL_CODE(Error);
     {
       buffer<int, 1> Buffer{OpenCLBuffer, MyQueue.get_context()};
@@ -113,6 +152,8 @@ int main() {
         Failed = true;
       }
     }
+    Error = clReleaseContext(OCLCtx);
+    CHECK_OCL_CODE(Error);
   }
   // Check host accessor
   {
@@ -121,10 +162,11 @@ int main() {
     cl_int Error = CL_SUCCESS;
 
     queue MyQueue;
+    cl_context OCLCtx = MyQueue.get_context().get();
 
-    cl_mem OpenCLBuffer = clCreateBuffer(
-        MyQueue.get_context().get(), CL_MEM_READ_WRITE | CL_MEM_COPY_HOST_PTR,
-        Size * sizeof(int), Init, &Error);
+    cl_mem OpenCLBuffer =
+        clCreateBuffer(OCLCtx, CL_MEM_READ_WRITE | CL_MEM_COPY_HOST_PTR,
+                       Size * sizeof(int), Init, &Error);
     CHECK_OCL_CODE(Error);
     buffer<int, 1> Buffer{OpenCLBuffer, MyQueue.get_context()};
 
@@ -144,6 +186,8 @@ int main() {
     }
     Error = clReleaseMemObject(OpenCLBuffer);
     CHECK_OCL_CODE(Error);
+    Error = clReleaseContext(OCLCtx);
+    CHECK_OCL_CODE(Error);
   }
   // Check interop constructor event
   {

sycl/unittests/scheduler/LinkedAllocaDependencies.cpp

@@ -33,6 +33,7 @@ class MemObjMock : public cl::sycl::detail::SYCLMemObjI {
   void releaseMem(ContextImplPtr, void *) {}
   void releaseHostMem(void *) {}
   size_t getSize() const override { return 10; }
+  detail::ContextImplPtr getInteropContext() const override { return nullptr; }
 };
 
 TEST_F(SchedulerTest, LinkedAllocaDependencies) {