From 1249c7c29e8d2d9cd5a341cc6be9df179c91b11c Mon Sep 17 00:00:00 2001 From: George Nash Date: Wed, 20 Oct 2021 13:10:31 -0700 Subject: [PATCH] Resolve issue when running Yolov4 on DNNL EP (#9355) The dnnl_binary ops need the memory format to match the format expected by Onnxruntime. If the memory format of the inputs do not match each other there will be an error in the calculated results. Additionally, since the code manually pads the tensor dimensions for broadcasting the inputs are expected to be in Onnxruntimes format. Since detecting and reordering the memory to Ort format matches what was previously done for the Reshape op the code was moved from dnnl_reshape to dnnl_subgraph_primitive under the name GetMemoryInOrtFormat. One small additional change made to the capability code log to also print the percentage of nodes run by the dnnl execution provider. Signed-off-by: George Nash --- .../providers/dnnl/dnnl_execution_provider.cc | 5 ++- .../providers/dnnl/subgraph/dnnl_binary.cc | 7 +++- .../providers/dnnl/subgraph/dnnl_reshape.cc | 36 +----------------- .../providers/dnnl/subgraph/dnnl_reshape.h | 1 - .../providers/dnnl/subgraph/dnnl_subgraph.cc | 2 +- .../providers/dnnl/subgraph/dnnl_subgraph.h | 2 +- .../dnnl/subgraph/dnnl_subgraph_primitive.cc | 38 +++++++++++++++++++ .../dnnl/subgraph/dnnl_subgraph_primitive.h | 5 +++ 8 files changed, 56 insertions(+), 40 deletions(-) diff --git a/onnxruntime/core/providers/dnnl/dnnl_execution_provider.cc b/onnxruntime/core/providers/dnnl/dnnl_execution_provider.cc index 7267886fc6..80a15ea1dc 100644 --- a/onnxruntime/core/providers/dnnl/dnnl_execution_provider.cc +++ b/onnxruntime/core/providers/dnnl/dnnl_execution_provider.cc @@ -11,6 +11,7 @@ #include "dnnl_fwd.h" #include "dnnl_node_capability.h" +#include #include #include "gsl/gsl" #define ORT_API_MANUAL_INIT @@ -217,10 +218,12 @@ std::vector> DNNLExecutionProvider::GetCapabi } if (debug_log_) { + float percent_dnnl = 100.0f * (static_cast(num_of_supported_nodes) / static_cast(graph_viewer.NumberOfNodes())); LOGS_DEFAULT(ERROR) << "DNNLExecutionProvider::GetCapability," << " number of partitions supported by DNNL: " << result.size() << " number of nodes in the graph: " << graph_viewer.NumberOfNodes() - << " number of nodes supported by DNNL: " << num_of_supported_nodes; + << " number of nodes supported by DNNL: " << num_of_supported_nodes + << std::fixed << std::setprecision(2) << " (" << percent_dnnl << "%)"; } if (dump_subgraphs_) { diff --git a/onnxruntime/core/providers/dnnl/subgraph/dnnl_binary.cc b/onnxruntime/core/providers/dnnl/subgraph/dnnl_binary.cc index a39dfe7cea..8e146dc578 100644 --- a/onnxruntime/core/providers/dnnl/subgraph/dnnl_binary.cc +++ b/onnxruntime/core/providers/dnnl/subgraph/dnnl_binary.cc @@ -23,8 +23,9 @@ void DnnlBinary::CreatePrimitive(DnnlSubgraphPrimitive& sp, DnnlNode& node) { ORT_THROW("op type not supported"); } - auto src_0_ori_md = sp.GetMemory(node.Input(IN_A)).get_desc(); - auto src_1_ori_md = sp.GetMemory(node.Input(IN_B)).get_desc(); + // GetMemory in OrtFormat. Broadcasting and mix format binary ops can result in computation failure + auto src_0_ori_md = sp.GetMemoryInOrtFormat(node.Input(IN_A), eng).get_desc(); + auto src_1_ori_md = sp.GetMemoryInOrtFormat(node.Input(IN_B), eng).get_desc(); auto src_0_dims = src_0_ori_md.dims(); auto src_1_dims = src_1_ori_md.dims(); @@ -55,6 +56,8 @@ void DnnlBinary::CreatePrimitive(DnnlSubgraphPrimitive& sp, DnnlNode& node) { auto binary_src0_mem = sp.GetMemoryAndReshape(node.Input(IN_A), binary_pd.src0_desc(), eng); auto binary_src1_mem = sp.GetMemoryAndReshape(node.Input(IN_B), binary_pd.src1_desc(), eng); + + auto binary_dst_mem = dnnl::memory(binary_pd.dst_desc(), eng); auto binary_prim = dnnl::binary(binary_pd); diff --git a/onnxruntime/core/providers/dnnl/subgraph/dnnl_reshape.cc b/onnxruntime/core/providers/dnnl/subgraph/dnnl_reshape.cc index a779d5400a..8759c821c9 100644 --- a/onnxruntime/core/providers/dnnl/subgraph/dnnl_reshape.cc +++ b/onnxruntime/core/providers/dnnl/subgraph/dnnl_reshape.cc @@ -13,21 +13,9 @@ DnnlReshape::DnnlReshape() { } void DnnlReshape::CreatePrimitive(DnnlSubgraphPrimitive& sp, DnnlNode& node) { auto dnnl_engine = sp.GetEngine(); - auto data_mem = sp.GetMemory(node.Input(IN_DATA)); + // the input shape assumes OrtFormat so we get the memory in OrtFormat. + auto data_mem = sp.GetMemoryInOrtFormat(node.Input(IN_DATA), dnnl_engine); dnnl::memory::dims data_dims = data_mem.get_desc().dims(); - auto data_md = data_mem.get_desc(); - - - if (!IsMemoryInExpectedOrtFormat(data_md)) { - auto temp_md = dnnl::memory::desc(data_dims, node.Input(IN_DATA).Type(), sp.GetDnnlFormat(data_dims.size())); - dnnl::memory temp_mem = dnnl::memory(temp_md, dnnl_engine); - sp.AddPrimitive(dnnl::reorder(data_mem, temp_mem), {{DNNL_ARG_FROM, data_mem}, - {DNNL_ARG_TO, temp_mem}}); - data_mem = temp_mem; - } else { - // If using GPU this will move the memory from the CPU to the GPU. - data_mem = sp.GetMemoryAndReshape(node.Input(IN_DATA), data_md, dnnl_engine); - } auto shape_mem = sp.GetMemory(node.Input(IN_SHAPE)); dnnl::memory::dims shape_dims = shape_mem.get_desc().dims(); @@ -47,26 +35,6 @@ void DnnlReshape::CreatePrimitive(DnnlSubgraphPrimitive& sp, DnnlNode& node) { sp.SetMemory(node.Output(OUT_RESHAPED), reshaped_mem, true); } -bool DnnlReshape::IsMemoryInExpectedOrtFormat(const dnnl::memory::desc& desc) { - if (desc.data.format_kind != dnnl_blocked) { - return false; - } - if (desc.data.format_desc.blocking.inner_nblks != 0) { - return false; - } - auto strides = desc.data.format_desc.blocking.strides; - // if a data format is dnnl_format::abcd... the stride will go from largest to smallest - // if for example we have a shape {2,3,4} we expect a stride of {12, 4, 1} if it were - // of dnnl_format::abc if instead the stride were {12, 1, 4} that would be dnnl_format::acb - // which does not match what is expected from Onnxruntime. - for (size_t i = 1; i < desc.dims().size(); ++i) { - if (strides[i - 1] < strides[i]) { - return false; - } - } - return true; -} - bool DnnlReshape::GetAllowZero(DnnlNode& node) { auto attr = node.Attributes().find("allowzero"); int64_t allowzero = 0; //Default value according to ONNX spec diff --git a/onnxruntime/core/providers/dnnl/subgraph/dnnl_reshape.h b/onnxruntime/core/providers/dnnl/subgraph/dnnl_reshape.h index 10c19db6d6..93bf00ce7d 100644 --- a/onnxruntime/core/providers/dnnl/subgraph/dnnl_reshape.h +++ b/onnxruntime/core/providers/dnnl/subgraph/dnnl_reshape.h @@ -23,7 +23,6 @@ class DnnlReshape{ void CreatePrimitive(DnnlSubgraphPrimitive& sp, DnnlNode& node); private: - bool IsMemoryInExpectedOrtFormat(const dnnl::memory::desc& desc); bool GetAllowZero(DnnlNode& node); }; diff --git a/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph.cc b/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph.cc index e1a82bf5e4..92c2fca5ed 100644 --- a/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph.cc +++ b/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph.cc @@ -45,7 +45,7 @@ dnnl::memory::dims DnnlTensor::Dim() { return dnnl_dims; } -dnnl::memory::data_type DnnlTensor::Type() { +dnnl::memory::data_type DnnlTensor::Type() const { auto data_type = arg_->TypeAsProto()->tensor_type().elem_type(); switch (data_type) { case ONNX_NAMESPACE::TensorProto_DataType_UNDEFINED: diff --git a/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph.h b/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph.h index d2df733bce..9615ace10c 100644 --- a/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph.h +++ b/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph.h @@ -18,7 +18,7 @@ class DnnlTensor { DnnlTensor(std::string name); std::string Name() const; dnnl::memory::dims Dim(); - dnnl::memory::data_type Type(); + dnnl::memory::data_type Type() const; dnnl::memory::format_tag Format(); //check whether the tensor is dynamic, e.g. contains unspecified dimension bool IsDynamic(); diff --git a/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph_primitive.cc b/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph_primitive.cc index e3a8029b30..6889684a8d 100644 --- a/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph_primitive.cc +++ b/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph_primitive.cc @@ -379,6 +379,8 @@ void DnnlSubgraphPrimitive::SetInitializer(std::string memory_name, dnnl::memory } } + + dnnl::memory DnnlSubgraphPrimitive::GetMemoryAndReshape(const DnnlTensor& tensor, dnnl::memory::desc mem_desc, dnnl::engine eng, bool transpose) { // if found just return if (HasMemory(tensor.Name(), mem_desc, eng)) { @@ -437,6 +439,42 @@ dnnl::memory DnnlSubgraphPrimitive::GetMemoryAndReshape(const DnnlTensor& tensor return mem_to; } +dnnl::memory DnnlSubgraphPrimitive::GetMemoryInOrtFormat(const DnnlTensor& tensor, const dnnl::engine& eng) { + auto from_mem = GetMemory(tensor); + auto from_desc = from_mem.get_desc(); + auto from_dims = from_desc.dims(); + if (!IsMemoryInExpectedOrtFormat(from_desc)) { + dnnl::memory::desc to_md = dnnl::memory::desc(from_dims, tensor.Type(), GetDnnlFormat(from_dims.size())); + dnnl::memory to_mem = dnnl::memory(to_md, eng); + AddPrimitive(dnnl::reorder(from_mem, to_mem), {{DNNL_ARG_FROM, from_mem}, + {DNNL_ARG_TO, to_mem}}); + return to_mem; + } else { + // If using GPU this will move the memory from the CPU to the GPU. + return GetMemoryAndReshape(tensor, from_desc, eng); + } +} + +bool DnnlSubgraphPrimitive::IsMemoryInExpectedOrtFormat(const dnnl::memory::desc& desc) const { + if (desc.data.format_kind != dnnl_blocked) { + return false; + } + if (desc.data.format_desc.blocking.inner_nblks != 0) { + return false; + } + auto strides = desc.data.format_desc.blocking.strides; + // if a data format is dnnl_format::abcd... the stride will go from largest to smallest + // if for example we have a shape {2,3,4} we expect a stride of {12, 4, 1} if it were + // of dnnl_format::abc if instead the stride were {12, 1, 4} that would be dnnl_format::acb + // which does not match what is expected from Onnxruntime. + for (size_t i = 1; i < desc.dims().size(); ++i) { + if (strides[i - 1] < strides[i]) { + return false; + } + } + return true; +} + void DnnlSubgraphPrimitive::AddReshape(dnnl::memory src, dnnl::memory dst) { LOGS_DEFAULT(INFO) << "reshape queued"; reshapes_.push_back({src, dst}); diff --git a/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph_primitive.h b/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph_primitive.h index a3afd7646c..490ab17a60 100644 --- a/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph_primitive.h +++ b/onnxruntime/core/providers/dnnl/subgraph/dnnl_subgraph_primitive.h @@ -63,6 +63,11 @@ class DnnlSubgraphPrimitive { bool IsDynamic(); OrtMutex& GetMutex() { return mutex_; } + //GetMemory in OrtFormat if the memory is not in the OrtFormat this will reorder the memory. + //All memory will be moved to the dnnl_engine even if it is already in OrtFormat. + dnnl::memory GetMemoryInOrtFormat(const DnnlTensor& tensor, const dnnl::engine& eng); + bool IsMemoryInExpectedOrtFormat(const dnnl::memory::desc& desc) const; + private: std::string shape_key_;