From ab2c35926f501f219f902d6db726186f90a120a8 Mon Sep 17 00:00:00 2001 From: Ben Tracy Date: Tue, 22 Oct 2024 13:48:50 +0100 Subject: [PATCH 1/3] [SYCL][Graph] Add unit test/mocking for command handle behaviour - Add a unit test and mocking functions that check the command handle behaviour changes from #15522 --- .../Extensions/CommandGraph/Update.cpp | 11 +++ sycl/unittests/helpers/UrMock.hpp | 67 +++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/sycl/unittests/Extensions/CommandGraph/Update.cpp b/sycl/unittests/Extensions/CommandGraph/Update.cpp index ff66af25d9e83..32a88433801af 100644 --- a/sycl/unittests/Extensions/CommandGraph/Update.cpp +++ b/sycl/unittests/Extensions/CommandGraph/Update.cpp @@ -399,3 +399,14 @@ TEST_F(WholeGraphUpdateTest, EmptyNode) { auto GraphExec = Graph.finalize(experimental::property::graph::updatable{}); GraphExec.update(UpdateGraph); } + +TEST_F(CommandGraphTest, CheckFinalizeBehaviour) { + // Check that both finalize with and without updatable property work as + // expected + auto Node = Graph.add( + [&](sycl::handler &cgh) { cgh.single_task>([]() {}); }); + + ASSERT_NO_THROW(Graph.finalize(experimental::property::graph::updatable{})); + + ASSERT_NO_THROW(Graph.finalize()); +} diff --git a/sycl/unittests/helpers/UrMock.hpp b/sycl/unittests/helpers/UrMock.hpp index 0beed5a06dd1c..53c4db96dd84d 100644 --- a/sycl/unittests/helpers/UrMock.hpp +++ b/sycl/unittests/helpers/UrMock.hpp @@ -439,6 +439,67 @@ inline ur_result_t mock_urVirtualMemReserve(void *pParams) { return UR_RESULT_SUCCESS; } +// Create dummy command buffer handle and store the provided descriptor as the +// data +inline ur_result_t mock_urCommandBufferCreateExp(void *pParams) { + auto params = + reinterpret_cast(pParams); + const ur_exp_command_buffer_desc_t *descPtr = *(params->ppCommandBufferDesc); + ur_exp_command_buffer_handle_t *retCmdBuffer = *params->pphCommandBuffer; + *retCmdBuffer = mock::createDummyHandle( + static_cast(sizeof(ur_exp_command_buffer_desc_t))); + if (descPtr) { + reinterpret_cast(*retCmdBuffer) + ->setDataAs(*descPtr); + } + + return UR_RESULT_SUCCESS; +} + +inline ur_result_t mock_urCommandBufferGetInfoExp(void *pParams) { + auto params = + reinterpret_cast(pParams); + + auto cmdBufferDummyHandle = + reinterpret_cast(*params->phCommandBuffer); + switch (*params->ppropName) { + case UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR: { + if (*params->ppPropValue) { + ur_exp_command_buffer_desc_t *propValue = + reinterpret_cast( + *params->ppPropValue); + *propValue = + cmdBufferDummyHandle->getDataAs(); + } + if (*params->ppPropSizeRet) + **params->ppPropSizeRet = sizeof(ur_exp_command_buffer_desc_t); + } + return UR_RESULT_SUCCESS; + default: + return UR_RESULT_SUCCESS; + } + return UR_RESULT_SUCCESS; +} + +// Checking command handle behaviour only +inline ur_result_t mock_urCommandBufferAppendKernelLaunchExp(void *pParams) { + auto params = + reinterpret_cast( + pParams); + + auto cmdBufferDummyHandle = + reinterpret_cast(*params->phCommandBuffer); + // Requesting a command handle when the command buffer is not updatable is an + // error + if (*(params->pphCommand) && + cmdBufferDummyHandle->getDataAs() + .isUpdatable == false) { + return UR_RESULT_ERROR_INVALID_OPERATION; + } + + return UR_RESULT_SUCCESS; +} + } // namespace MockAdapter /// The UrMock<> class sets up UR for adapter mocking with the set of default @@ -484,6 +545,12 @@ template class UrMock { ADD_DEFAULT_OVERRIDE(urUsmP2PPeerAccessGetInfoExp, mock_urUsmP2PPeerAccessGetInfoExp) ADD_DEFAULT_OVERRIDE(urVirtualMemReserve, mock_urVirtualMemReserve) + ADD_DEFAULT_OVERRIDE(urCommandBufferCreateExp, + mock_urCommandBufferCreateExp); + ADD_DEFAULT_OVERRIDE(urCommandBufferAppendKernelLaunchExp, + mock_urCommandBufferAppendKernelLaunchExp); + ADD_DEFAULT_OVERRIDE(urCommandBufferGetInfoExp, + mock_urCommandBufferGetInfoExp); #undef ADD_DEFAULT_OVERRIDE ur_loader_config_handle_t UrLoaderConfig = nullptr; From 3a2cef8fd0f399d6615e7c582efa33afbb4fa70a Mon Sep 17 00:00:00 2001 From: Ben Tracy Date: Tue, 22 Oct 2024 16:07:13 +0100 Subject: [PATCH 2/3] Update sycl/unittests/Extensions/CommandGraph/Update.cpp Co-authored-by: Ewan Crawford --- sycl/unittests/Extensions/CommandGraph/Update.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sycl/unittests/Extensions/CommandGraph/Update.cpp b/sycl/unittests/Extensions/CommandGraph/Update.cpp index 32a88433801af..da0b398818445 100644 --- a/sycl/unittests/Extensions/CommandGraph/Update.cpp +++ b/sycl/unittests/Extensions/CommandGraph/Update.cpp @@ -400,7 +400,7 @@ TEST_F(WholeGraphUpdateTest, EmptyNode) { GraphExec.update(UpdateGraph); } -TEST_F(CommandGraphTest, CheckFinalizeBehaviour) { +TEST_F(CommandGraphTest, CheckFinalizeBehavior) { // Check that both finalize with and without updatable property work as // expected auto Node = Graph.add( From 60f1e494019e4e2eb5362dc00aafaebdeb8bcbd8 Mon Sep 17 00:00:00 2001 From: Ben Tracy Date: Thu, 31 Oct 2024 12:58:16 +0000 Subject: [PATCH 3/3] Add function call count checking to new finalize behaviour test --- .../Extensions/CommandGraph/Update.cpp | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/sycl/unittests/Extensions/CommandGraph/Update.cpp b/sycl/unittests/Extensions/CommandGraph/Update.cpp index da0b398818445..94296507996dd 100644 --- a/sycl/unittests/Extensions/CommandGraph/Update.cpp +++ b/sycl/unittests/Extensions/CommandGraph/Update.cpp @@ -400,13 +400,37 @@ TEST_F(WholeGraphUpdateTest, EmptyNode) { GraphExec.update(UpdateGraph); } +// Vars and callbacks for tracking how many times mocked functions are called +static int GetInfoCount = 0; +static int AppendKernelLaunchCount = 0; +static ur_result_t redefinedCommandBufferGetInfoExpAfter(void *pParams) { + GetInfoCount++; + return UR_RESULT_SUCCESS; +} +static ur_result_t +redefinedCommandBufferAppendKernelLaunchExpAfter(void *pParams) { + AppendKernelLaunchCount++; + return UR_RESULT_SUCCESS; +} + TEST_F(CommandGraphTest, CheckFinalizeBehavior) { // Check that both finalize with and without updatable property work as // expected auto Node = Graph.add( [&](sycl::handler &cgh) { cgh.single_task>([]() {}); }); + mock::getCallbacks().set_after_callback( + "urCommandBufferGetInfoExp", &redefinedCommandBufferGetInfoExpAfter); + mock::getCallbacks().set_after_callback( + "urCommandBufferAppendKernelLaunchExp", + &redefinedCommandBufferAppendKernelLaunchExpAfter); ASSERT_NO_THROW(Graph.finalize(experimental::property::graph::updatable{})); + // GetInfo and AppendKernelLaunch should be called once each time a node is + // added to a command buffer during finalization + ASSERT_EQ(GetInfoCount, 1); + ASSERT_EQ(AppendKernelLaunchCount, 1); ASSERT_NO_THROW(Graph.finalize()); + ASSERT_EQ(GetInfoCount, 2); + ASSERT_EQ(AppendKernelLaunchCount, 2); }