Add missing retain and fix some null adapter mentions.

aarongreig · kbenzie · commit c90c6d04b9e8 · 2024-07-11T10:50:11.000+01:00
diff --git a/examples/collector/README.md b/examples/collector/README.md
@@ -19,7 +19,7 @@ $ mkdir build
 $ cd build
 $ cmake .. -DUR_ENABLE_TRACING=ON
 $ make
-$ UR_ADAPTERS_FORCE_LOAD=./lib/libur_adapter_null.so UR_ENABLE_LAYERS=UR_LAYER_TRACING XPTI_TRACE_ENABLE=1 XPTI_FRAMEWORK_DISPATCHER=./lib/libxptifw.so XPTI_SUBSCRIBERS=./lib/libcollector.so ./bin/hello_world
+$ UR_ADAPTERS_FORCE_LOAD=./lib/libur_adapter_mock.so UR_ENABLE_LAYERS=UR_LAYER_TRACING XPTI_TRACE_ENABLE=1 XPTI_FRAMEWORK_DISPATCHER=./lib/libxptifw.so XPTI_SUBSCRIBERS=./lib/libcollector.so ./bin/hello_world
 ```
 
 See [XPTI framework documentation](https://github.com/intel/llvm/blob/sycl/xptifw/doc/XPTI_Framework.md) for more information.
diff --git a/include/ur_api.h b/include/ur_api.h
@@ -8822,7 +8822,7 @@ urCommandBufferEnqueueExp(
 ///     - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
 UR_APIEXPORT ur_result_t UR_APICALL
 urCommandBufferRetainCommandExp(
-    ur_exp_command_buffer_command_handle_t hCommand ///< [in] Handle of the command-buffer command.
+    ur_exp_command_buffer_command_handle_t hCommand ///< [in][retain] Handle of the command-buffer command.
 );
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/scripts/core/exp-command-buffer.yml b/scripts/core/exp-command-buffer.yml
@@ -879,7 +879,7 @@ name: RetainCommandExp
 params:
     - type: $x_exp_command_buffer_command_handle_t
       name: hCommand
-      desc: "[in] Handle of the command-buffer command."
+      desc: "[in][retain] Handle of the command-buffer command."
 returns:
     - $X_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP
     - $X_RESULT_ERROR_OUT_OF_RESOURCES
diff --git a/source/adapters/mock/ur_mockddi.cpp b/source/adapters/mock/ur_mockddi.cpp
@@ -1,6 +1,6 @@
 /*
  *
- * Copyright (C) 2019-2024 Intel Corporation
+ * Copyright (C) 2024 Intel Corporation
  *
  * Part of the Unified-Runtime Project, under the Apache License v2.0 with LLVM Exceptions.
  * See LICENSE.TXT
@@ -9139,7 +9139,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferEnqueueExp(
 /// @brief Intercept function for urCommandBufferRetainCommandExp
 __urdlllocal ur_result_t UR_APICALL urCommandBufferRetainCommandExp(
     ur_exp_command_buffer_command_handle_t
-        hCommand ///< [in] Handle of the command-buffer command.
+        hCommand ///< [in][retain] Handle of the command-buffer command.
     ) try {
     ur_result_t result = UR_RESULT_SUCCESS;
 
@@ -9161,6 +9161,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferRetainCommandExp(
         result = replaceCallback(&params);
     } else {
 
+        mock::retainDummyHandle(hCommand);
         result = UR_RESULT_SUCCESS;
     }
 
diff --git a/source/loader/layers/tracing/ur_trcddi.cpp b/source/loader/layers/tracing/ur_trcddi.cpp
@@ -7135,7 +7135,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferEnqueueExp(
 /// @brief Intercept function for urCommandBufferRetainCommandExp
 __urdlllocal ur_result_t UR_APICALL urCommandBufferRetainCommandExp(
     ur_exp_command_buffer_command_handle_t
-        hCommand ///< [in] Handle of the command-buffer command.
+        hCommand ///< [in][retain] Handle of the command-buffer command.
 ) {
     auto pfnRetainCommandExp =
         context.urDdiTable.CommandBufferExp.pfnRetainCommandExp;
diff --git a/source/loader/layers/validation/ur_valddi.cpp b/source/loader/layers/validation/ur_valddi.cpp
@@ -8722,7 +8722,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferEnqueueExp(
 /// @brief Intercept function for urCommandBufferRetainCommandExp
 __urdlllocal ur_result_t UR_APICALL urCommandBufferRetainCommandExp(
     ur_exp_command_buffer_command_handle_t
-        hCommand ///< [in] Handle of the command-buffer command.
+        hCommand ///< [in][retain] Handle of the command-buffer command.
 ) {
     auto pfnRetainCommandExp =
         context.urDdiTable.CommandBufferExp.pfnRetainCommandExp;
diff --git a/source/loader/ur_ldrddi.cpp b/source/loader/ur_ldrddi.cpp
@@ -7492,7 +7492,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferEnqueueExp(
 /// @brief Intercept function for urCommandBufferRetainCommandExp
 __urdlllocal ur_result_t UR_APICALL urCommandBufferRetainCommandExp(
     ur_exp_command_buffer_command_handle_t
-        hCommand ///< [in] Handle of the command-buffer command.
+        hCommand ///< [in][retain] Handle of the command-buffer command.
 ) {
     ur_result_t result = UR_RESULT_SUCCESS;
 
@@ -7514,6 +7514,12 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferRetainCommandExp(
     // forward to device-platform
     result = pfnRetainCommandExp(hCommand);
 
+    if (UR_RESULT_SUCCESS != result) {
+        return result;
+    }
+
+    // TODO: do we need to ref count the loader handles?
+
     return result;
 }
 
diff --git a/source/loader/ur_libapi.cpp b/source/loader/ur_libapi.cpp
@@ -8173,7 +8173,7 @@ ur_result_t UR_APICALL urCommandBufferEnqueueExp(
 ///     - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
 ur_result_t UR_APICALL urCommandBufferRetainCommandExp(
     ur_exp_command_buffer_command_handle_t
-        hCommand ///< [in] Handle of the command-buffer command.
+        hCommand ///< [in][retain] Handle of the command-buffer command.
     ) try {
     auto pfnRetainCommandExp =
         ur_lib::context->urDdiTable.CommandBufferExp.pfnRetainCommandExp;
diff --git a/source/ur_api.cpp b/source/ur_api.cpp
@@ -6928,7 +6928,7 @@ ur_result_t UR_APICALL urCommandBufferEnqueueExp(
 ///     - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
 ur_result_t UR_APICALL urCommandBufferRetainCommandExp(
     ur_exp_command_buffer_command_handle_t
-        hCommand ///< [in] Handle of the command-buffer command.
+        hCommand ///< [in][retain] Handle of the command-buffer command.
 ) {
     ur_result_t result = UR_RESULT_SUCCESS;
     return result;
diff --git a/test/conformance/CMakeLists.txt b/test/conformance/CMakeLists.txt
@@ -87,7 +87,7 @@ function(add_conformance_test name)
     if(NOT (UR_BUILD_ADAPTER_CUDA OR UR_BUILD_ADAPTER_HIP
             OR UR_BUILD_ADAPTER_L0 OR UR_BUILD_ADAPTER_OPENCL
             OR UR_BUILD_ADAPTER_NATIVE_CPU OR UR_BUILD_ADAPTER_ALL))
-        add_test_adapter(${name} adapter_null)
+        add_test_adapter(${name} adapter_mock)
     endif()
 endfunction()
 
diff --git a/test/fuzz/README.md b/test/fuzz/README.md
@@ -9,7 +9,7 @@ which provides the path where any new corpus will be saved. The path has to exis
 It's worth running the test with tracing enabled while picking scenarios to be added to the repository
 for future short fuzz tests runs. Example of running the test with generating new corpus files:
 ```
-UR_ADAPTERS_FORCE_LOAD=build/lib/libur_adapter_null.so \
+UR_ADAPTERS_FORCE_LOAD=build/lib/libur_adapter_mock.so \
 XPTI_TRACE_ENABLE=1 \
 XPTI_FRAMEWORK_DISPATCHER=build/lib/libxptifw.so \
 XPTI_SUBSCRIBERS=build/lib/libcollector.so \
@@ -19,7 +19,7 @@ UR_ENABLE_LAYERS=UR_LAYER_TRACING \
 
 Pass path to a corpus file instead to run a single scenario:
 ```
-UR_ADAPTERS_FORCE_LOAD=build/lib/libur_adapter_null.so \
+UR_ADAPTERS_FORCE_LOAD=build/lib/libur_adapter_mock.so \
 XPTI_TRACE_ENABLE=1 \
 XPTI_FRAMEWORK_DISPATCHER=build/lib/libxptifw.so \
 XPTI_SUBSCRIBERS=build/lib/libcollector.so \
diff --git a/tools/urtrace/urtrace.py b/tools/urtrace/urtrace.py
@@ -40,13 +40,13 @@ def get_dynamic_library_name(name):
     epilog='''examples:
 
     %(prog)s ./myapp --myapp-arg
-    %(prog)s --null --profiling --filter ".*(Device|Platform).*" ./hello_world
+    %(prog)s --mock --profiling --filter ".*(Device|Platform).*" ./hello_world
     %(prog)s --adapter libur_adapter_cuda.so --begin ./sycl_app''',
     formatter_class=argparse.RawDescriptionHelpFormatter)
 parser.add_argument("command", help="Command to run, including arguments.", nargs=argparse.REMAINDER)
 parser.add_argument("--profiling", help="Measure function execution time.", action="store_true")
 parser.add_argument("--filter", help="Only trace functions that match the provided regex filter.")
-parser.add_argument("--null", help="Force the use of the null adapter.", action="store_true")
+parser.add_argument("--mock", help="Force the use of the mock adapter.", action="store_true")
 parser.add_argument("--adapter", help="Force the use of the provided adapter.", action="append", default=[])
 parser.add_argument("--json", help="Write output in a JSON Trace Event Format.", action="store_true")
 group = parser.add_mutually_exclusive_group()
@@ -113,12 +113,12 @@ def get_dynamic_library_name(name):
 
 force_load = None
 
-if args.null:
-    null_lib = get_dynamic_library_name("ur_adapter_null")
-    null_adapter = find_library(args.libpath, null_lib, args.recursive)
-    if null_adapter is None:
-        sys.exit("unable to find the null adapter - " + null_lib)
-    force_load = "\"" + null_adapter + "\""
+if args.mock:
+    mock_lib = get_dynamic_library_name("ur_adapter_mock")
+    mock_adapter = find_library(args.libpath, mock_lib, args.recursive)
+    if mock_adapter is None:
+        sys.exit("unable to find the mock adapter - " + mock_lib)
+    force_load = "\"" + mock_adapter + "\""
 
 for adapter in args.adapter:
     adapter_path = find_library(args.libpath, adapter, args.recursive) if is_filename(adapter) else adapter
