From d1e8d4a2613f25823d997a38d51b138ca9adcedb Mon Sep 17 00:00:00 2001 From: Hector Li Date: Thu, 8 Jun 2023 17:09:35 -0700 Subject: [PATCH] [QNN EP] Fix an issue for Conv with dynamic weights (#16235) ### Description Fix an issue for Conv with dynamic weights Root cause: Conv op builder create the weight input tensor with wrong name. With dynamic weight, Transpose node is inserted. Conv op builder should use the new name which is Transpose output. It cause the weight producer has wrong output shape. --- .../qnn/builder/opbuilder/conv_op_builder.cc | 12 ++-- .../qnn/builder/opbuilder/gemm_op_builder.cc | 2 +- .../qnn/builder/qnn_backend_manager.cc | 1 + .../qnn/builder/qnn_model_wrapper.cc | 5 +- .../providers/qnn/builder/qnn_model_wrapper.h | 7 +- onnxruntime/test/providers/qnn/conv_test.cc | 67 +++++++++++++++++++ 6 files changed, 84 insertions(+), 10 deletions(-) diff --git a/onnxruntime/core/providers/qnn/builder/opbuilder/conv_op_builder.cc b/onnxruntime/core/providers/qnn/builder/opbuilder/conv_op_builder.cc index 5da223fa23..f7c1ae9a8d 100644 --- a/onnxruntime/core/providers/qnn/builder/opbuilder/conv_op_builder.cc +++ b/onnxruntime/core/providers/qnn/builder/opbuilder/conv_op_builder.cc @@ -53,7 +53,6 @@ class ConvOpBuilder : public BaseOpBuilder { // The nodes from 1st call of GetCapability do not get layout transformer applied, it's still NCHW // The nodes from 2nd call of GetCapability get layout transformer applied, it's NHWC // Need to do op validation in 1st call of GetCapability -// TODO: Check if node domain == kMSInternalNHWCDomain to determine if the layout has been transformed. Status ConvOpBuilder::IsOpSupported(QnnModelWrapper& qnn_model_wrapper, const NodeUnit& node_unit, const logging::Logger& logger, @@ -128,7 +127,6 @@ Status ConvOpBuilder::ProcessInputs(QnnModelWrapper& qnn_model_wrapper, bool is_quantized_model, std::vector& input_names, bool do_op_validation) const { - ORT_UNUSED_PARAMETER(do_op_validation); Qnn_QuantizeParams_t quantize_param = QNN_QUANTIZE_PARAMS_INIT; InitializeQuantizeParam(quantize_param, is_quantized_model); Qnn_DataType_t qnn_data_type = QNN_DATATYPE_FLOAT_32; @@ -192,6 +190,7 @@ Status ConvOpBuilder::ProcessInputs(QnnModelWrapper& qnn_model_wrapper, new_input_shape, qnn_data_type, quantize_param, + do_op_validation, is_graph_input)); } else if (node_unit.OpType() == "ConvTranspose") { ORT_RETURN_IF_ERROR(qnn_model_wrapper.AddCnhwToHwcnTranspose(node_unit.Index(), @@ -201,6 +200,7 @@ Status ConvOpBuilder::ProcessInputs(QnnModelWrapper& qnn_model_wrapper, new_input_shape, qnn_data_type, quantize_param, + do_op_validation, is_graph_input)); } else { ORT_THROW("Unexpected operator %s", node_unit.OpType()); @@ -211,14 +211,14 @@ Status ConvOpBuilder::ProcessInputs(QnnModelWrapper& qnn_model_wrapper, } input_names.push_back(input_tensor_name); - if (qnn_model_wrapper.IsQnnTensorWrapperExist(input_name)) { - LOGS(logger, VERBOSE) << "Tensor already added, skip it: " << input_name; + if (qnn_model_wrapper.IsQnnTensorWrapperExist(input_tensor_name)) { + LOGS(logger, VERBOSE) << "Tensor already added, skip it: " << input_tensor_name; continue; } - Qnn_TensorType_t tensor_type = GetInputTensorType(qnn_model_wrapper, input_name); + Qnn_TensorType_t tensor_type = GetInputTensorType(qnn_model_wrapper, input_tensor_name); - QnnTensorWrapper input_tensorwrapper(input_name, tensor_type, qnn_data_type, quantize_param, + QnnTensorWrapper input_tensorwrapper(input_tensor_name, tensor_type, qnn_data_type, quantize_param, std::move(input_shape), std::move(unpacked_tensor)); ORT_RETURN_IF_NOT(qnn_model_wrapper.AddTensorWrapper(std::move(input_tensorwrapper)), "Failed to add tensor."); } diff --git a/onnxruntime/core/providers/qnn/builder/opbuilder/gemm_op_builder.cc b/onnxruntime/core/providers/qnn/builder/opbuilder/gemm_op_builder.cc index a4ea16555b..d100338e29 100644 --- a/onnxruntime/core/providers/qnn/builder/opbuilder/gemm_op_builder.cc +++ b/onnxruntime/core/providers/qnn/builder/opbuilder/gemm_op_builder.cc @@ -134,7 +134,7 @@ Status GemmOpBuilder::ProcessInputs(QnnModelWrapper& qnn_model_wrapper, std::vector perm{1, 0}; ORT_RETURN_IF_ERROR(qnn_model_wrapper.AddTransposeNode(node_unit.Index(), node_input_name, input_tensor_name, old_input_shape, perm, input_shape, - qnn_data_type, quantize_param)); + qnn_data_type, quantize_param, do_op_validation)); } if (2 == input_i && 2 == input_shape.size()) { diff --git a/onnxruntime/core/providers/qnn/builder/qnn_backend_manager.cc b/onnxruntime/core/providers/qnn/builder/qnn_backend_manager.cc index b3b05fd5e5..ebee7aa4b5 100644 --- a/onnxruntime/core/providers/qnn/builder/qnn_backend_manager.cc +++ b/onnxruntime/core/providers/qnn/builder/qnn_backend_manager.cc @@ -170,6 +170,7 @@ void QnnBackendManager::InitializeQnnLog() { default: break; } + LOGS(*logger_, VERBOSE) << "Set Qnn log level: " << qnn_log_level; if (QNN_SUCCESS != qnn_interface_.logCreate(QnnLogging, qnn_log_level, &log_handle_)) { LOGS(*logger_, WARNING) << "Unable to initialize logging in the QNN backend."; diff --git a/onnxruntime/core/providers/qnn/builder/qnn_model_wrapper.cc b/onnxruntime/core/providers/qnn/builder/qnn_model_wrapper.cc index 62a199efee..45eae1df51 100644 --- a/onnxruntime/core/providers/qnn/builder/qnn_model_wrapper.cc +++ b/onnxruntime/core/providers/qnn/builder/qnn_model_wrapper.cc @@ -63,6 +63,7 @@ bool QnnModelWrapper::AddTensorWrapper(QnnTensorWrapper&& tensor_wrapper) { } if (IsQnnTensorWrapperExist(tensor_name) == true) { + LOGS(logger_, VERBOSE) << "Tensor eist already: " << tensor_name; return true; } @@ -362,6 +363,7 @@ Status QnnModelWrapper::AddTransposeNode(NodeIndex node_index, const std::vector& output_shape, const Qnn_DataType_t& tensor_data_type, const Qnn_QuantizeParams_t& quantize_param, + bool do_op_validation, const bool is_for_input, const bool is_for_output) { // No need to add this for output nodes as it is added as output tensor for previous node @@ -397,7 +399,8 @@ Status QnnModelWrapper::AddTransposeNode(NodeIndex node_index, qnn_node_type, {input_name}, {output_name}, - {param_tensor_name}); + {param_tensor_name}, + do_op_validation); return Status::OK(); } diff --git a/onnxruntime/core/providers/qnn/builder/qnn_model_wrapper.h b/onnxruntime/core/providers/qnn/builder/qnn_model_wrapper.h index 93a2860951..9a0c605252 100644 --- a/onnxruntime/core/providers/qnn/builder/qnn_model_wrapper.h +++ b/onnxruntime/core/providers/qnn/builder/qnn_model_wrapper.h @@ -113,6 +113,7 @@ class QnnModelWrapper { const std::vector& output_shape, const Qnn_DataType_t& tensor_data_type, const Qnn_QuantizeParams_t& quantize_param, + bool do_op_validation, const bool is_for_input = true, const bool is_for_output = false); @@ -124,12 +125,13 @@ class QnnModelWrapper { const std::vector& output_shape, const Qnn_DataType_t& tensor_data_type, const Qnn_QuantizeParams_t& quantize_param, + bool do_op_validation, const bool is_for_input = true, const bool is_for_output = false) { LOGS(logger_, VERBOSE) << "Add NCHW->HWCN Transpose node after Conv weight input: " << input_name << " -> " << output_name; return AddTransposeNode(node_index, input_name, output_name, input_shape, nchw2hwcn_perm_, output_shape, - tensor_data_type, quantize_param, is_for_input, is_for_output); + tensor_data_type, quantize_param, do_op_validation, is_for_input, is_for_output); } // Tranpose CNHW->HWCN for QNN weight @@ -140,12 +142,13 @@ class QnnModelWrapper { const std::vector& output_shape, const Qnn_DataType_t& tensor_data_type, const Qnn_QuantizeParams_t& quantize_param, + bool do_op_validation, const bool is_for_input = true, const bool is_for_output = false) { LOGS(logger_, VERBOSE) << "Add CNHW->HWCN Transpose node after ConvTranspose weight input: " << input_name << " -> " << output_name; return AddTransposeNode(node_index, input_name, output_name, input_shape, cnhw2hwcn_perm_, output_shape, - tensor_data_type, quantize_param, is_for_input, is_for_output); + tensor_data_type, quantize_param, do_op_validation, is_for_input, is_for_output); } Status UnpackInitializerData(const ONNX_NAMESPACE::TensorProto& initializer, diff --git a/onnxruntime/test/providers/qnn/conv_test.cc b/onnxruntime/test/providers/qnn/conv_test.cc index e4e3f756b0..7c3e715539 100644 --- a/onnxruntime/test/providers/qnn/conv_test.cc +++ b/onnxruntime/test/providers/qnn/conv_test.cc @@ -13,6 +13,73 @@ namespace onnxruntime { namespace test { +// The bug is from a QDQ model, and Conv node gets processed before it's producer Mul node +// A Transpose node gets inserted between Mul and the dynamic weight tensor shape on Conv +// to make Conv weight with shape HWNC +// However it changes Mul output shape to HWNC and cause issue +// It has to be QDQ model, because the DQ node with initializer on Conv gets processed first +// and DQ node requires its node unit to be processed +// So, Conv gets processed before Mul node +TEST_F(QnnCPUBackendTests, Test_QDQConvWithDynamicWeightsFromMul) { + ProviderOptions provider_options; + +#if defined(_WIN32) + provider_options["backend_path"] = "QnnHtp.dll"; +#else + provider_options["backend_path"] = "libQnnHtp.so"; +#endif + + auto BuildConvMulGraph = [](ModelTestBuilder& builder) { + // DQ node for Conv input + auto* dq_i_output = builder.MakeIntermediate(); + auto* conv_dq_input = builder.MakeInitializer({1, 32, 16, 113}, static_cast(0), static_cast(127)); + + // DQ node for Conv bias + auto* dq_bias_output = builder.MakeIntermediate(); + auto* bias = builder.MakeInitializer({16}, static_cast(0), static_cast(127)); + + // Mul node + // DQ nodes for Mul + auto* mul_dq1_output = builder.MakeIntermediate(); + auto* mul_input1 = builder.MakeInput({16, 32, 1, 1}, static_cast(0), static_cast(127)); + + auto* mul_dq2_output = builder.MakeIntermediate(); + auto* mul_input2 = builder.MakeInitializer({16, 1, 1, 1}, static_cast(0), static_cast(127)); + builder.AddDequantizeLinearNode(mul_input1, .03f, 0, mul_dq1_output); + builder.AddDequantizeLinearNode(mul_input2, .03f, 0, mul_dq2_output); + + auto* mul_output = builder.MakeIntermediate(); + builder.AddNode("Mul", {mul_dq1_output, mul_dq2_output}, {mul_output}); + + auto* mul_dq_output = AddQDQNodePair(builder, mul_output, .03f, 0); + + builder.AddDequantizeLinearNode(conv_dq_input, .04f, 0, dq_i_output); + builder.AddDequantizeLinearNode(bias, .0012f, 0, dq_bias_output); + // Conv node + auto* conv_output = builder.MakeIntermediate(); + + Node& conv_node = builder.AddNode("Conv", {dq_i_output, mul_dq_output, dq_bias_output}, {conv_output}); + conv_node.AddAttribute("auto_pad", "NOTSET"); + conv_node.AddAttribute("pads", std::vector{0, 0, 0, 0}); + conv_node.AddAttribute("strides", std::vector{1, 1}); + conv_node.AddAttribute("dilations", std::vector{1, 1}); + + auto* q_output = builder.MakeIntermediate(); + builder.AddQuantizeLinearNode(conv_output, .039f, 0, q_output); + + auto* dq_output = builder.MakeOutput(); + builder.AddDequantizeLinearNode(q_output, .039f, 0, dq_output); + }; + + constexpr int expected_nodes_in_partition = 1; + RunQnnModelTest(BuildConvMulGraph, + provider_options, + 13, + ExpectedEPNodeAssignment::All, + expected_nodes_in_partition, + "Test_ConvWithDynamicWeightsFromMul"); +} + // Creates a graph with a single Conv operator. Used for testing CPU backend. static GetTestModelFn BuildConvTestCase(const std::vector& input_shape, const std::vector& weights_shape,