Merge branch 'main' into user-after-free

AllanZyne · AllanZyne · commit 3d9aa6438d89 · 2024-03-13T23:13:08.000-07:00
diff --git a/source/adapters/cuda/command_buffer.cpp b/source/adapters/cuda/command_buffer.cpp
@@ -368,7 +368,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
   try {
     // Set node param structure with the kernel related data
     auto &ArgIndices = hKernel->getArgIndices();
-    CUDA_KERNEL_NODE_PARAMS NodeParams;
+    CUDA_KERNEL_NODE_PARAMS NodeParams = {};
     NodeParams.func = CuFunc;
     NodeParams.gridDimX = BlocksPerGrid[0];
     NodeParams.gridDimY = BlocksPerGrid[1];
@@ -378,8 +378,6 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
     NodeParams.blockDimZ = ThreadsPerBlock[2];
     NodeParams.sharedMemBytes = LocalSize;
     NodeParams.kernelParams = const_cast<void **>(ArgIndices.data());
-    NodeParams.kern = nullptr;
-    NodeParams.extra = nullptr;
 
     // Create and add an new kernel node to the Cuda graph
     UR_CHECK_ERROR(cuGraphAddKernelNode(&GraphNode, hCommandBuffer->CudaGraph,
diff --git a/source/adapters/level_zero/device.cpp b/source/adapters/level_zero/device.cpp
@@ -1054,6 +1054,19 @@ bool ur_device_handle_t_::useRelaxedAllocationLimits() {
   return EnableRelaxedAllocationLimits;
 }
 
+bool ur_device_handle_t_::useDriverInOrderLists() {
+  // Use in-order lists implementation from L0 driver instead
+  // of adapter's implementation.
+  static const bool UseDriverInOrderLists = [] {
+    const char *UrRet = std::getenv("UR_L0_USE_DRIVER_INORDER_LISTS");
+    if (!UrRet)
+      return false;
+    return std::atoi(UrRet) != 0;
+  }();
+
+  return UseDriverInOrderLists;
+}
+
 ur_result_t ur_device_handle_t_::initialize(int SubSubDeviceOrdinal,
                                             int SubSubDeviceIndex) {
   // Maintain various device properties cache.
diff --git a/source/adapters/level_zero/device.hpp b/source/adapters/level_zero/device.hpp
@@ -143,6 +143,9 @@ struct ur_device_handle_t_ : _ur_object {
   // Read env settings to select immediate commandlist mode.
   ImmCmdlistMode useImmediateCommandLists();
 
+  // Whether Adapter uses driver's implementation of in-order lists or not
+  bool useDriverInOrderLists();
+
   // Returns whether immediate command lists are used on this device.
   ImmCmdlistMode ImmCommandListUsed{};
 
diff --git a/source/adapters/level_zero/event.cpp b/source/adapters/level_zero/event.cpp
@@ -43,6 +43,20 @@ static const bool UseMultipleCmdlistBarriers = [] {
   return std::atoi(UseMultipleCmdlistBarriersFlag) > 0;
 }();
 
+bool WaitListEmptyOrAllEventsFromSameQueue(
+    ur_queue_handle_t Queue, uint32_t NumEventsInWaitList,
+    const ur_event_handle_t *EventWaitList) {
+  if (!NumEventsInWaitList)
+    return true;
+
+  for (uint32_t i = 0; i < NumEventsInWaitList; ++i) {
+    if (Queue != EventWaitList[i]->UrQueue)
+      return false;
+  }
+
+  return true;
+}
+
 UR_APIEXPORT ur_result_t UR_APICALL urEnqueueEventsWait(
     ur_queue_handle_t Queue,      ///< [in] handle of the queue object
     uint32_t NumEventsInWaitList, ///< [in] size of the event wait list
@@ -206,21 +220,11 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueEventsWaitWithBarrier(
   bool IsInternal = OutEvent == nullptr;
   ur_event_handle_t *Event = OutEvent ? OutEvent : &InternalEvent;
 
-  auto WaitListEmptyOrAllEventsFromSameQueue = [Queue, NumEventsInWaitList,
-                                                EventWaitList]() {
-    if (!NumEventsInWaitList)
-      return true;
-
-    for (uint32_t I = 0; I < NumEventsInWaitList; ++I)
-      if (Queue != EventWaitList[I]->UrQueue)
-        return false;
-
-    return true;
-  };
-
   // For in-order queue and wait-list which is empty or has events from
   // the same queue just use the last command event as the barrier event.
-  if (Queue->isInOrderQueue() && WaitListEmptyOrAllEventsFromSameQueue() &&
+  if (Queue->isInOrderQueue() &&
+      WaitListEmptyOrAllEventsFromSameQueue(Queue, NumEventsInWaitList,
+                                            EventWaitList) &&
       Queue->LastCommandEvent && !Queue->LastCommandEvent->IsDiscarded) {
     UR_CALL(urEventRetain(Queue->LastCommandEvent));
     *Event = Queue->LastCommandEvent;
@@ -1189,6 +1193,23 @@ ur_result_t _ur_ze_event_list_t::createAndRetainUrZeEventList(
       CurQueue->LastCommandEvent && CurQueue->LastCommandEvent->IsDiscarded)
     IncludeLastCommandEvent = false;
 
+  // If we are using L0 native implementation for handling in-order queues,
+  // then we don't need to add the last enqueued event into the waitlist, as
+  // the native driver implementation will already ensure in-order semantics.
+  // The only exception is when a different immediate command was last used on
+  // the same UR Queue.
+  if (CurQueue->Device->useDriverInOrderLists() && CurQueue->isInOrderQueue() &&
+      CurQueue->UsingImmCmdLists) {
+    auto QueueGroup = CurQueue->getQueueGroup(UseCopyEngine);
+    uint32_t QueueGroupOrdinal, QueueIndex;
+    auto NextIndex = QueueGroup.getQueueIndex(&QueueGroupOrdinal, &QueueIndex,
+                                              /*QueryOnly */ true);
+    auto NextImmCmdList = QueueGroup.ImmCmdLists[NextIndex];
+    IncludeLastCommandEvent &=
+        CurQueue->LastUsedCommandList != CurQueue->CommandListMap.end() &&
+        NextImmCmdList != CurQueue->LastUsedCommandList;
+  }
+
   try {
     uint32_t TmpListLength = 0;
 
@@ -1205,6 +1226,16 @@ ur_result_t _ur_ze_event_list_t::createAndRetainUrZeEventList(
       this->UrEventList = new ur_event_handle_t[EventListLength];
     }
 
+    // For in-order queue and wait-list which is empty or has events only from
+    // the same queue then we don't need to wait on any other additional events
+    if (CurQueue->Device->useDriverInOrderLists() &&
+        CurQueue->isInOrderQueue() &&
+        WaitListEmptyOrAllEventsFromSameQueue(CurQueue, EventListLength,
+                                              EventList)) {
+      this->Length = TmpListLength;
+      return UR_RESULT_SUCCESS;
+    }
+
     if (EventListLength > 0) {
       for (uint32_t I = 0; I < EventListLength; I++) {
         {
diff --git a/source/adapters/level_zero/image.cpp b/source/adapters/level_zero/image.cpp
@@ -738,13 +738,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesSampledImageCreateExp(
       hContext, hDevice, hImageMem, pImageFormat, pImageDesc, phMem, phImage));
 
   struct combined_sampled_image_handle {
-    uint64_t raw_image_handle;
-    uint64_t raw_sampler_handle;
+    uint64_t RawImageHandle;
+    uint64_t RawSamplerHandle;
   };
-  combined_sampled_image_handle *sampledImageHandle =
+  auto *SampledImageHandle =
       reinterpret_cast<combined_sampled_image_handle *>(phImage);
-  sampledImageHandle->raw_image_handle = reinterpret_cast<uint64_t>(*phImage);
-  sampledImageHandle->raw_sampler_handle =
+  SampledImageHandle->RawSamplerHandle =
       reinterpret_cast<uint64_t>(hSampler->ZeSampler);
 
   return UR_RESULT_SUCCESS;
diff --git a/source/adapters/level_zero/kernel.cpp b/source/adapters/level_zero/kernel.cpp
@@ -214,8 +214,8 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueKernelLaunch(
   // the code can do a urKernelRelease on this kernel.
   (*Event)->CommandData = (void *)Kernel;
 
-  // Increment the reference count of the Kernel and indicate that the Kernel is
-  // in use. Once the event has been signalled, the code in
+  // Increment the reference count of the Kernel and indicate that the Kernel
+  // is in use. Once the event has been signalled, the code in
   // CleanupCompletedEvent(Event) will do a urKernelRelease to update the
   // reference count on the kernel, using the kernel saved in CommandData.
   UR_CALL(urKernelRetain(Kernel));
diff --git a/source/adapters/level_zero/queue.cpp b/source/adapters/level_zero/queue.cpp
@@ -1870,6 +1870,10 @@ ur_result_t ur_queue_handle_t_::createCommandList(
   ZeStruct<ze_command_list_desc_t> ZeCommandListDesc;
   ZeCommandListDesc.commandQueueGroupOrdinal = QueueGroupOrdinal;
 
+  if (Device->useDriverInOrderLists() && isInOrderQueue()) {
+    ZeCommandListDesc.flags = ZE_COMMAND_LIST_FLAG_IN_ORDER;
+  }
+
   ZE2UR_CALL(zeCommandListCreate, (Context->ZeContext, Device->ZeDevice,
                                    &ZeCommandListDesc, &ZeCommandList));
 
@@ -1985,7 +1989,11 @@ ur_command_list_ptr_t &ur_queue_handle_t_::ur_queue_group_t::getImmCmdList() {
 
   // Evaluate performance of explicit usage for "0" index.
   if (QueueIndex != 0) {
-    ZeCommandQueueDesc.flags = ZE_COMMAND_QUEUE_FLAG_EXPLICIT_ONLY;
+    ZeCommandQueueDesc.flags |= ZE_COMMAND_QUEUE_FLAG_EXPLICIT_ONLY;
+  }
+
+  if (Queue->Device->useDriverInOrderLists() && Queue->isInOrderQueue()) {
+    ZeCommandQueueDesc.flags |= ZE_COMMAND_QUEUE_FLAG_IN_ORDER;
   }
 
   // Check if context's command list cache has an immediate command list with
diff --git a/source/loader/ur_lib.cpp b/source/loader/ur_lib.cpp
@@ -224,10 +224,6 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
     if (!hPlatform) {
         return UR_RESULT_ERROR_INVALID_NULL_HANDLE;
     }
-    // NumEntries is max number of devices wanted by the caller (max usable length of phDevices)
-    if (NumEntries < 0) {
-        return UR_RESULT_ERROR_INVALID_SIZE;
-    }
     if (NumEntries > 0 && !phDevices) {
         return UR_RESULT_ERROR_INVALID_NULL_POINTER;
     }
@@ -426,8 +422,10 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
 
     for (auto &termPair : mapODS) {
         std::string backend = termPair.first;
-        if (backend
-                .empty()) { // FIXME: never true because getenv_to_map rejects this case
+        // TODO: Figure out how to process all ODS errors rather than returning
+        // on the first error.
+        if (backend.empty()) {
+            // FIXME: never true because getenv_to_map rejects this case
             // malformed term: missing backend -- output ERROR, then continue
             logger::error("ERROR: missing backend, format of filter = "
                           "'[!]backend:filterStrings'");
@@ -459,20 +457,19 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
                                    std::tolower(static_cast<unsigned char>(b));
                         })) {
             // irrelevant term for current request: different backend -- silently ignore
-            logger::warning(
-                "WARNING: ignoring term with irrelevant backend '{}'", backend);
-            continue;
+            logger::error("unrecognised backend '{}'", backend);
+            return UR_RESULT_ERROR_INVALID_VALUE;
         }
         if (termPair.second.size() == 0) {
-            // malformed term: missing filterStrings -- output ERROR, then continue
-            logger::error("ERROR missing filterStrings, format of filter = "
+            // malformed term: missing filterStrings -- output ERROR
+            logger::error("missing filterStrings, format of filter = "
                           "'[!]backend:filterStrings'");
-            continue;
+            return UR_RESULT_ERROR_INVALID_VALUE;
         }
         if (std::find_if(termPair.second.cbegin(), termPair.second.cend(),
                          [](const auto &s) { return s.empty(); }) !=
-            termPair.second
-                .cend()) { // FIXME: never true because getenv_to_map rejects this case
+            termPair.second.cend()) {
+            // FIXME: never true because getenv_to_map rejects this case
             // malformed term: missing filterString -- output warning, then continue
             logger::warning(
                 "WARNING: empty filterString, format of filterStrings "
@@ -483,10 +480,10 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
                          [](const auto &s) {
                              return std::count(s.cbegin(), s.cend(), '.') > 2;
                          }) != termPair.second.cend()) {
-            // malformed term: too many dots in filterString -- output warning, then continue
-            logger::warning("WARNING: too many dots in filterString, format of "
-                            "filterString = 'root[.sub[.subsub]]'");
-            continue;
+            // malformed term: too many dots in filterString
+            logger::error("too many dots in filterString, format of "
+                          "filterString = 'root[.sub[.subsub]]'");
+            return UR_RESULT_ERROR_INVALID_VALUE;
         }
         if (std::find_if(
                 termPair.second.cbegin(), termPair.second.cend(),
@@ -504,10 +501,9 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
                     }
                     return false; // no BAD things, so must be okay
                 }) != termPair.second.cend()) {
-            // malformed term: star dot no-star in filterString -- output warning, then continue
-            logger::warning(
-                "WARNING: invalid wildcard in filterString, '*.' => '*.*'");
-            continue;
+            // malformed term: star dot no-star in filterString
+            logger::error("invalid wildcard in filterString, '*.' => '*.*'");
+            return UR_RESULT_ERROR_INVALID_VALUE;
         }
 
         // TODO -- use regex validation_pattern to catch all other syntax errors in the ODS string
@@ -552,7 +548,7 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
 
     if (acceptDeviceList.size() == 0 && discardDeviceList.size() == 0) {
         // nothing in env var was understood as a valid term
-        return UR_RESULT_ERROR_INVALID_VALUE;
+        return UR_RESULT_SUCCESS;
     } else if (acceptDeviceList.size() == 0) {
         // no accept terms were understood, but at least one discard term was
         // we are magnanimous to the user when there were bad/ignored accept terms
diff --git a/test/conformance/device/urDeviceGet.cpp b/test/conformance/device/urDeviceGet.cpp
@@ -35,6 +35,42 @@ TEST_F(urDeviceGetTest, SuccessSubsetOfDevices) {
     }
 }
 
+struct urDeviceGetTestWithDeviceTypeParam
+    : uur::urAllDevicesTest,
+      ::testing::WithParamInterface<ur_device_type_t> {
+
+    void SetUp() override {
+        UUR_RETURN_ON_FATAL_FAILURE(uur::urAllDevicesTest::SetUp());
+    }
+};
+
+INSTANTIATE_TEST_SUITE_P(
+    , urDeviceGetTestWithDeviceTypeParam,
+    ::testing::Values(UR_DEVICE_TYPE_DEFAULT, UR_DEVICE_TYPE_GPU,
+                      UR_DEVICE_TYPE_CPU, UR_DEVICE_TYPE_FPGA,
+                      UR_DEVICE_TYPE_MCA, UR_DEVICE_TYPE_VPU),
+    [](const ::testing::TestParamInfo<ur_device_type_t> &info) {
+        std::stringstream ss;
+        ss << info.param;
+        return ss.str();
+    });
+
+TEST_P(urDeviceGetTestWithDeviceTypeParam, Success) {
+    ur_device_type_t device_type = GetParam();
+    uint32_t count = 0;
+    ASSERT_SUCCESS(urDeviceGet(platform, device_type, 0, nullptr, &count));
+    ASSERT_GE(devices.size(), count);
+
+    if (count > 0) {
+        std::vector<ur_device_handle_t> devices(count);
+        ASSERT_SUCCESS(
+            urDeviceGet(platform, device_type, count, devices.data(), nullptr));
+        for (auto device : devices) {
+            ASSERT_NE(nullptr, device);
+        }
+    }
+}
+
 TEST_F(urDeviceGetTest, InvalidNullHandlePlatform) {
     uint32_t count;
     ASSERT_EQ_RESULT(
diff --git a/test/conformance/enqueue/urEnqueueMemBufferMap.cpp b/test/conformance/enqueue/urEnqueueMemBufferMap.cpp
@@ -21,14 +21,21 @@ TEST_P(urEnqueueMemBufferMapTest, SuccessRead) {
     }
 }
 
-TEST_P(urEnqueueMemBufferMapTest, SuccessWrite) {
+using urEnqueueMemBufferMapTestWithWriteFlagParam =
+    uur::urMemBufferQueueTestWithParam<ur_map_flag_t>;
+UUR_TEST_SUITE_P(urEnqueueMemBufferMapTestWithWriteFlagParam,
+                 ::testing::Values(UR_MAP_FLAG_WRITE,
+                                   UR_MAP_FLAG_WRITE_INVALIDATE_REGION),
+                 uur::deviceTestWithParamPrinter<ur_map_flag_t>);
+
+TEST_P(urEnqueueMemBufferMapTestWithWriteFlagParam, SuccessWrite) {
     const std::vector<uint32_t> input(count, 0);
     ASSERT_SUCCESS(urEnqueueMemBufferWrite(queue, buffer, true, 0, size,
                                            input.data(), 0, nullptr, nullptr));
 
     uint32_t *map = nullptr;
-    ASSERT_SUCCESS(urEnqueueMemBufferMap(queue, buffer, true, UR_MAP_FLAG_WRITE,
-                                         0, size, 0, nullptr, nullptr,
+    ASSERT_SUCCESS(urEnqueueMemBufferMap(queue, buffer, true, getParam(), 0,
+                                         size, 0, nullptr, nullptr,
                                          (void **)&map));
     for (unsigned i = 0; i < count; ++i) {
         map[i] = 42;
diff --git a/test/conformance/memory/urMemBufferCreate.cpp b/test/conformance/memory/urMemBufferCreate.cpp
@@ -48,6 +48,18 @@ UUR_TEST_SUITE_P(urMemBufferCreateWithHostPtrFlagsTest,
                                    UR_MEM_FLAG_USE_HOST_POINTER),
                  uur::deviceTestWithParamPrinter<ur_mem_flag_t>);
 
+TEST_P(urMemBufferCreateWithHostPtrFlagsTest, SUCCESS) {
+    uur::raii::Mem host_ptr_buffer = nullptr;
+    ASSERT_SUCCESS(urMemBufferCreate(context, UR_MEM_FLAG_ALLOC_HOST_POINTER,
+                                     4096, nullptr, host_ptr_buffer.ptr()));
+
+    ur_buffer_properties_t properties{UR_STRUCTURE_TYPE_BUFFER_PROPERTIES,
+                                      nullptr, host_ptr_buffer.ptr()};
+    uur::raii::Mem buffer = nullptr;
+    ASSERT_SUCCESS(urMemBufferCreate(context, getParam(), 4096, &properties,
+                                     buffer.ptr()));
+}
+
 TEST_P(urMemBufferCreateWithHostPtrFlagsTest, InvalidHostPtr) {
     uur::raii::Mem buffer = nullptr;
     ASSERT_EQ_RESULT(
diff --git a/test/conformance/platform/urPlatformGetInfo.cpp b/test/conformance/platform/urPlatformGetInfo.cpp
@@ -19,7 +19,7 @@ INSTANTIATE_TEST_SUITE_P(
     urPlatformGetInfo, urPlatformGetInfoTest,
     ::testing::Values(UR_PLATFORM_INFO_NAME, UR_PLATFORM_INFO_VENDOR_NAME,
                       UR_PLATFORM_INFO_VERSION, UR_PLATFORM_INFO_EXTENSIONS,
-                      UR_PLATFORM_INFO_PROFILE),
+                      UR_PLATFORM_INFO_PROFILE, UR_PLATFORM_INFO_BACKEND),
     [](const ::testing::TestParamInfo<ur_platform_info_t> &info) {
         std::stringstream ss;
         ss << info.param;
@@ -30,11 +30,17 @@ TEST_P(urPlatformGetInfoTest, Success) {
     size_t size = 0;
     ur_platform_info_t info_type = GetParam();
     ASSERT_SUCCESS(urPlatformGetInfo(platform, info_type, 0, nullptr, &size));
-    ASSERT_NE(size, 0);
+    if (info_type == UR_PLATFORM_INFO_BACKEND) {
+        ASSERT_EQ(size, sizeof(ur_platform_backend_t));
+    } else {
+        ASSERT_NE(size, 0);
+    }
     std::vector<char> name(size);
     ASSERT_SUCCESS(
         urPlatformGetInfo(platform, info_type, size, name.data(), nullptr));
-    ASSERT_EQ(size, std::strlen(name.data()) + 1);
+    if (info_type != UR_PLATFORM_INFO_BACKEND) {
+        ASSERT_EQ(size, std::strlen(name.data()) + 1);
+    }
 }
 
 TEST_P(urPlatformGetInfoTest, InvalidNullHandlePlatform) {
diff --git a/test/conformance/program/urProgramBuild.cpp b/test/conformance/program/urProgramBuild.cpp
diff --git a/test/conformance/program/urProgramCreateWithIL.cpp b/test/conformance/program/urProgramCreateWithIL.cpp
diff --git a/test/conformance/queue/queue_adapter_native_cpu.match b/test/conformance/queue/queue_adapter_native_cpu.match
diff --git a/test/conformance/queue/urQueueCreate.cpp b/test/conformance/queue/urQueueCreate.cpp
diff --git a/test/conformance/testing/include/uur/fixtures.h b/test/conformance/testing/include/uur/fixtures.h
diff --git a/tools/urinfo/urinfo.cpp b/tools/urinfo/urinfo.cpp
