Respond to comments

Author: Hugh Delaney

sycl/include/sycl/detail/pi.h

@@ -1405,6 +1405,7 @@ __SYCL_EXPORT pi_result piextMemGetNativeHandle(pi_mem mem,
 /// Gets the native handle of a PI mem object on a particular device
 ///
 /// \param mem is the PI mem to get the native handle of.
+/// \param dev is the PI device that the native allocation will be resident on.
 /// \param nativeHandle is the native handle of mem.
 __SYCL_EXPORT pi_result piextMemGetNativeHandleExp(
     pi_mem mem, pi_device dev, pi_native_handle *nativeHandle);

sycl/test-e2e/HostInteropTask/interop-task-hip.cpp

@@ -1,5 +1,5 @@
 // FIXME: the rocm include path and link path are highly platform dependent,
-// we should set this with some variable instead
+// we should set this with some variable instead.
 // RUN: %{build} -o %t.out -I/opt/rocm/include -L/opt/rocm/lib -lamdhip64
 // RUN: %{run} %t.out
 // REQUIRES: hip
@@ -42,7 +42,6 @@ void copy(buffer<DataT, 1> &Src, buffer<DataT, 1> &Dst, queue &Q) {
       auto HipStream = IH.get_native_queue<backend::ext_oneapi_hip>();
       auto SrcMem = IH.get_native_mem<backend::ext_oneapi_hip>(SrcA);
       auto DstMem = IH.get_native_mem<backend::ext_oneapi_hip>(DstA);
-      cl_event Event;
 
       if (hipMemcpyWithStream(DstMem, SrcMem, sizeof(DataT) * SrcA.get_count(),
                               hipMemcpyDefault, HipStream) != hipSuccess) {
@@ -77,14 +76,14 @@ void init(buffer<DataT, 1> &B1, buffer<DataT, 1> &B2, queue &Q) {
     auto Acc2 = B2.template get_access<mode::write>(CGH);
 
     CGH.parallel_for<Init<DataT>>(BUFFER_SIZE, [=](item<1> Id) {
-      Acc1[Id] = -1;
-      Acc2[Id] = -2;
+      Acc1[Id] = B1Init;
+      Acc2[Id] = B2Init;
     });
   });
 }
 
-// A test that uses OpenCL interop to copy data from buffer A to buffer B, by
-// getting cl_mem objects and calling the clEnqueueBufferCopy. Then run a SYCL
+// A test that uses HIP interop to copy data from buffer A to buffer B, by
+// getting HIP ptrs and calling the hipMemcpyWithStream. Then run a SYCL
 // kernel that modifies the data in place for B, e.g. increment one, then copy
 // back to buffer A. Run it on a loop, to ensure the dependencies and the
 // reference counting of the objects is not leaked.
@@ -93,10 +92,10 @@ void test1(queue &Q) {
   buffer<int, 1> Buffer1{BUFFER_SIZE};
   buffer<int, 1> Buffer2{BUFFER_SIZE};
 
-  // init the buffer with a'priori invalid data
+  // Init the buffer with a'priori invalid data.
   init<int, -1, -2>(Buffer1, Buffer2, Q);
 
-  // Repeat a couple of times
+  // Repeat a couple of times.
   for (size_t Idx = 0; Idx < COUNT; ++Idx) {
     copy(Buffer1, Buffer2, Q);
     modify(Buffer2, Q);
@@ -115,10 +114,10 @@ void test2(queue &Q) {
   buffer<int, 1> Buffer1{BUFFER_SIZE};
   buffer<int, 1> Buffer2{BUFFER_SIZE};
 
-  // init the buffer with a'priori invalid data
+  // Init the buffer with a'priori invalid data.
   init<int, -1, -2>(Buffer1, Buffer2, Q);
 
-  // Repeat a couple of times
+  // Repeat a couple of times.
   for (size_t Idx = 0; Idx < COUNT; ++Idx) {
     copy(Buffer1, Buffer2, Q);
     modify(Buffer2, Q);
@@ -128,28 +127,7 @@ void test2(queue &Q) {
   checkBufferValues(Buffer2, COUNT - 1);
 }
 
-// Same as above but with queue constructed out of context
-void test2_1(queue &Q) {
-  static constexpr int COUNT = 4;
-  buffer<int, 1> Buffer1{BUFFER_SIZE};
-  buffer<int, 1> Buffer2{BUFFER_SIZE};
-
-  device Device;
-  auto Context = context(Device);
-  // init the buffer with a'priori invalid data
-  init<int, -1, -2>(Buffer1, Buffer2, Q);
-
-  // Repeat a couple of times
-  for (size_t Idx = 0; Idx < COUNT; ++Idx) {
-    copy(Buffer1, Buffer2, Q);
-    modify(Buffer2, Q);
-    copy(Buffer2, Buffer1, Q);
-  }
-  checkBufferValues(Buffer1, COUNT - 1);
-  checkBufferValues(Buffer2, COUNT - 1);
-}
-
-// Check that a single host-interop-task with a buffer will work
+// Check that a single host-interop-task with a buffer will work.
 void test3(queue &Q) {
   buffer<int, 1> Buffer{BUFFER_SIZE};
 
@@ -179,7 +157,6 @@ void test4(queue &Q) {
 void tests(queue &Q) {
   test1(Q);
   test2(Q);
-  test2_1(Q);
   test3(Q);
   test4(Q);
 }
@@ -193,7 +170,6 @@ int main() {
     std::rethrow_exception(*ExceptionList.begin());
   });
   tests(Q);
-  tests(Q);
   std::cout << "Test PASSED" << std::endl;
   return 0;
 }