Skip to content

Commit

Permalink
[QNN EP] Fix segfault when unregistering HTP shared memory handles (#…
Browse files Browse the repository at this point in the history
…23402)

### Description
- Fixes segfault when the function that cleans up HTP memory handles
uses an invalid Logger.
- Fixes unit test that compares output from QNN EP with exact float
values. QNN HTP runs float32 models with float16 precision, so need to
use a tolerance in the comparison.



### Motivation and Context
Fixes issues with using QNN HTP memory sharing on Windows ARM64. This is
also needed to test HTP shared memory with
#23120.
  • Loading branch information
adrianlizarraga authored Jan 17, 2025
1 parent 80f686e commit 2ab51c7
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,17 @@ Status QnnContextMemHandleManager::GetOrRegister(void* shared_memory_address, co

LOGS(logger_, VERBOSE) << "Registered QNN mem handle. mem_handle: " << raw_mem_handle;

const auto unregister_mem_handle = [this](Qnn_MemHandle_t raw_mem_handle) {
LOGS(logger_, VERBOSE) << "Unregistering QNN mem handle. mem_handle: " << raw_mem_handle;

const auto unregister_result = qnn_interface_.memDeRegister(&raw_mem_handle, 1);
// NOTE: Must use the default ORT logger inside this lambda. Don't capture this->logger_ because it may be deleted
// by the time we need to unregister all memory handles. This happens when this->logger_ is a session logger:
// ~InferenceSession() -> ~Logger() -> ~QnnExecutionProvider() -> ~QnnBackendManager() ->
// ~QnnContextMemHandleManager() -> unregister_mem_handle() segfault
const auto unregister_mem_handle = [&qnn_interface = this->qnn_interface_](Qnn_MemHandle_t raw_mem_handle) {
LOGS_DEFAULT(VERBOSE) << "Unregistering QNN mem handle. mem_handle: " << raw_mem_handle;

const auto unregister_result = qnn_interface.memDeRegister(&raw_mem_handle, 1);
if (unregister_result != QNN_SUCCESS) {
LOGS(logger_, ERROR) << "qnn_interface.memDeRegister() failed: "
<< utils::GetVerboseQnnErrorMessage(qnn_interface_, unregister_result);
LOGS_DEFAULT(ERROR) << "qnn_interface.memDeRegister() failed: "
<< utils::GetVerboseQnnErrorMessage(qnn_interface, unregister_result);
}
};

Expand Down
6 changes: 6 additions & 0 deletions onnxruntime/test/providers/qnn/qnn_basic_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,12 @@ TEST_F(QnnHTPBackendTests, UseHtpSharedMemoryAllocatorForInputs) {
qnn_ep = QnnExecutionProviderWithOptions(provider_options);
} catch (const OnnxRuntimeException& e) {
// handle particular exception that indicates that the libcdsprpc.so / dll can't be loaded
// NOTE: To run this on a local Windows ARM64 device, you need to copy libcdsprpc.dll to the build directory:
// - Open File Explorer
// - Go to C:/Windows/System32/DriverStore/FileRepository/
// - Search for a folder that begins with qcnspmcdm8380.inf_arm64_ and open it
// - Copy the libcdsprpc.dll into the build/[PATH CONTAINING onnxruntime.dll] directory of the application.
// TODO(adrianlizarraga): Update CMake build for unittests to automatically copy libcdsprpc.dll into build directory
#if defined(_WIN32)
constexpr const char* expected_error_message = "Failed to load libcdsprpc.dll";
#else
Expand Down
15 changes: 12 additions & 3 deletions onnxruntime/test/shared_lib/test_inference.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1960,6 +1960,12 @@ static bool CreateSessionWithQnnEpAndQnnHtpSharedMemoryAllocator(PATH_TYPE model
return true;
} catch (const Ort::Exception& e) {
// handle particular exception that indicates that the libcdsprpc.so / dll can't be loaded
// NOTE: To run this on a local Windows ARM64 device, you need to copy libcdsprpc.dll to the build directory:
// - Open File Explorer
// - Go to C:/Windows/System32/DriverStore/FileRepository/
// - Search for a folder that begins with qcnspmcdm8380.inf_arm64_ and open it
// - Copy the libcdsprpc.dll into the build/[PATH CONTAINING onnxruntime.dll] directory of the application.
// TODO(adrianlizarraga): Update CMake build for unittests to automatically copy libcdsprpc.dll into build directory
std::string_view error_message = e.what();

#if defined(_WIN32)
Expand Down Expand Up @@ -2275,8 +2281,11 @@ TEST(CApiTest, io_binding_qnn_htp_shared) {
Ort::Value bound_x = Ort::Value::CreateTensor(info_qnn_htp_shared, reinterpret_cast<float*>(input_data.get()), x_values.size(),
x_shape.data(), x_shape.size());

// Setup expected output (y) from model. Note that QNN EP runs float32 operators as float16,
// so the output will not be exactly equal.
const std::array<int64_t, 2> expected_y_shape = {3, 2};
const std::array<float, 3 * 2> expected_y = {1.0f, 4.0f, 9.0f, 16.0f, 25.0f, 36.0f};
constexpr float y_max_abs_err = 1e-5f;
auto output_data = qnn_htp_shared_allocator.GetAllocation(expected_y.size() * sizeof(float));
ASSERT_NE(output_data.get(), nullptr);

Expand All @@ -2293,7 +2302,7 @@ TEST(CApiTest, io_binding_qnn_htp_shared) {
// Check the values against the bound raw memory
{
gsl::span y{reinterpret_cast<const float*>(output_data.get()), expected_y.size()};
ASSERT_TRUE(std::equal(std::begin(y), std::end(y), std::begin(expected_y)));
EXPECT_THAT(expected_y, ::testing::Pointwise(::testing::FloatNear(y_max_abs_err), y));
}

// Now compare values via GetOutputValues
Expand All @@ -2308,7 +2317,7 @@ TEST(CApiTest, io_binding_qnn_htp_shared) {
ASSERT_EQ(expected_y.size(), count);

gsl::span y{Y_value.GetTensorData<float>(), count};
ASSERT_TRUE(std::equal(std::begin(y), std::end(y), std::begin(expected_y)));
EXPECT_THAT(expected_y, ::testing::Pointwise(::testing::FloatNear(y_max_abs_err), y));
}

{
Expand Down Expand Up @@ -2336,7 +2345,7 @@ TEST(CApiTest, io_binding_qnn_htp_shared) {
ASSERT_EQ(expected_y.size(), count);

gsl::span y{Y_value.GetTensorData<float>(), count};
ASSERT_TRUE(std::equal(std::begin(y), std::end(y), std::begin(expected_y)));
EXPECT_THAT(expected_y, ::testing::Pointwise(::testing::FloatNear(y_max_abs_err), y));
}

// Clean up
Expand Down

0 comments on commit 2ab51c7

Please sign in to comment.