From e380fd3c6beb1628ac1ee54d49ec0999832cc58b Mon Sep 17 00:00:00 2001 From: Jeff Bloomfield Date: Mon, 26 Oct 2020 22:16:22 +0000 Subject: [PATCH] Merged PR 5334334: Fix asserts and failure in GraphKernelHelper.cpp This extends a workaround needed to match node inputs with Tensors to the EP code handling constant input upload. This was causing issues in a couple of models, including EfficientDet, although that model still fails due to this bug: https://microsoft.visualstudio.com/OS/_workitems/edit/29970551 Related work items: #29706035 --- .../src/GraphDescBuilder.cpp | 41 ++--------------- .../src/GraphKernelHelper.cpp | 46 +++++++++++++++++-- .../src/GraphKernelHelper.h | 2 + 3 files changed, 49 insertions(+), 40 deletions(-) diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphDescBuilder.cpp b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphDescBuilder.cpp index 9cd8f92ae2..2c0fb0f374 100644 --- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphDescBuilder.cpp +++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphDescBuilder.cpp @@ -3,43 +3,12 @@ #include "precomp.h" #include "GraphDescBuilder.h" +#include "GraphKernelHelper.h" using namespace Windows::AI::MachineLearning::Adapter; namespace Dml::GraphDescBuilder { - // TODO: This is a hack which strips the suffix added within Lotus transforms that insert mem copies. - // This shouldn't be necessary if Lotus exposes the inputs/ouputs in the same order between the kernel - // for a function, and the graph for that function exposed as a kernel property. When the ordering - // mismatch is fixed (WindowsAI: 21114358, Lotus: 1953), this workaround should be removed. - static std::string GetFusedNodeArgNameMatchingGraph(const std::string& fusedNodeArgeName) - { - const char* suffix = nullptr; - - // The suffix used when inserting mem copies is equal to the below, probably followed by an incrementing number. - if (!suffix) { - suffix = strstr(fusedNodeArgeName.c_str(), "_DmlExecutionProvider_"); - } - - // The suffix used when inserting mem copies is equal to the below, not followed by an incrementing number. - if (!suffix) { - suffix = strstr(fusedNodeArgeName.c_str(), "_DmlExecutionProvider"); - } - - if (!suffix) { - suffix = strstr(fusedNodeArgeName.c_str(), "_token_"); - } - - if (suffix) - { - return std::string( - fusedNodeArgeName.begin(), - fusedNodeArgeName.begin() + (suffix - fusedNodeArgeName.c_str()) - ); - } else { - return fusedNodeArgeName; - } - } const std::string& GetUniqueNodeName(const onnxruntime::Node& node) { @@ -85,7 +54,7 @@ namespace Dml::GraphDescBuilder for (size_t inputIndex = 0; inputIndex < fusedNodeInputDefs.size(); ++inputIndex) { const onnxruntime::NodeArg* graphInput = graph.GetNodeArg( - GetFusedNodeArgNameMatchingGraph(fusedNodeInputDefs[inputIndex]->Name())); + GraphKernelHelper::GetFusedNodeArgNameMatchingGraph(fusedNodeInputDefs[inputIndex]->Name())); if (!graphInput) { @@ -208,10 +177,10 @@ namespace Dml::GraphDescBuilder auto iter = nameToFusedNodeInputIndex.find(arg->Name()); // The graph input could be missing the suffix, so try to match without it. - // This is part of a temporary workaround; see comments in GetFusedNodeArgNameMatchingGraph. + // This is part of a temporary workaround; see comments in GraphKernelHelper::GetFusedNodeArgNameMatchingGraph. if (iter == nameToFusedNodeInputIndex.end()) { - iter = nameToFusedNodeInputIndex.find(GetFusedNodeArgNameMatchingGraph(arg->Name())); + iter = nameToFusedNodeInputIndex.find(GraphKernelHelper::GetFusedNodeArgNameMatchingGraph(arg->Name())); } if (iter != nameToFusedNodeInputIndex.end()) @@ -277,7 +246,7 @@ namespace Dml::GraphDescBuilder for (size_t outputIndex = 0; outputIndex < fusedNodeOutputDefs.size(); ++outputIndex) { const onnxruntime::NodeArg* graphOutput = graph.GetNodeArg( - GetFusedNodeArgNameMatchingGraph(fusedNodeOutputDefs[outputIndex]->Name())); + GraphKernelHelper::GetFusedNodeArgNameMatchingGraph(fusedNodeOutputDefs[outputIndex]->Name())); const auto& outputNodeAndIndex = nameToNodeAndIndexMap.at(graphOutput->Name()); diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphKernelHelper.cpp b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphKernelHelper.cpp index f6aa742ce0..92895d0814 100644 --- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphKernelHelper.cpp +++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphKernelHelper.cpp @@ -109,7 +109,7 @@ namespace GraphKernelHelper const std::unordered_map& transferredInitializerMap) { // Transferred initializers are uploaded to GPU memory - auto iter = transferredInitializerMap.find(fusedNodeInputDefs[index]->Name()); + auto iter = transferredInitializerMap.find(GetFusedNodeArgNameMatchingGraph(fusedNodeInputDefs[index]->Name())); if (iter != transferredInitializerMap.end()) { return true; @@ -154,7 +154,7 @@ namespace GraphKernelHelper std::map initializerToLastInputIndexMap; for (uint32_t i = 0; i < graphInputCount; i++) { - auto iter = transferredInitializerMap.find(fusedNodeInputDefs[i]->Name()); + auto iter = transferredInitializerMap.find(GetFusedNodeArgNameMatchingGraph(fusedNodeInputDefs[i]->Name())); if (iter != transferredInitializerMap.end()) { initializerToLastInputIndexMap[&iter->second] = i; } @@ -172,7 +172,7 @@ namespace GraphKernelHelper // initialization or execution). So just throw away the transferred initializer and skip this input. if (!inputsUsed[i]) { - transferredInitializerMap.erase(fusedNodeInputDefs[i]->Name()); + transferredInitializerMap.erase(GetFusedNodeArgNameMatchingGraph(fusedNodeInputDefs[i]->Name())); if (inputRawData) { @@ -183,7 +183,7 @@ namespace GraphKernelHelper } // Look for the initializer among those transferred from the graph during partitioning - auto iter = transferredInitializerMap.find(fusedNodeInputDefs[i]->Name()); + auto iter = transferredInitializerMap.find(GetFusedNodeArgNameMatchingGraph(fusedNodeInputDefs[i]->Name())); if (iter != transferredInitializerMap.end()) { std::byte* tensorPtr = nullptr; @@ -320,5 +320,43 @@ namespace GraphKernelHelper dmlGraphDesc.IntermediateEdgeCount = gsl::narrow_cast(dmlIntermediateEdges.size()); dmlGraphDesc.IntermediateEdges = dmlIntermediateEdges.data(); } + + // TODO: This is a hack which strips the suffix added within Lotus transforms that insert mem copies. + // This shouldn't be necessary if Lotus exposes the inputs/ouputs in the same order between the kernel + // for a function, and the graph for that function exposed as a kernel property. When the ordering + // mismatch is fixed (WindowsAI: 21114358, Lotus: 1953), this workaround should be removed. + std::string GetFusedNodeArgNameMatchingGraph(const std::string& fusedNodeArgeName) + { + const char* suffix = nullptr; + + // The suffix used when inserting mem copies is equal to the below, probably followed by an incrementing number. + if (!suffix) + { + suffix = strstr(fusedNodeArgeName.c_str(), "_DmlExecutionProvider_"); + } + + // The suffix used when inserting mem copies is equal to the below, not followed by an incrementing number. + if (!suffix) + { + suffix = strstr(fusedNodeArgeName.c_str(), "_DmlExecutionProvider"); + } + + if (!suffix) + { + suffix = strstr(fusedNodeArgeName.c_str(), "_token_"); + } + + if (suffix) + { + return std::string( + fusedNodeArgeName.begin(), + fusedNodeArgeName.begin() + (suffix - fusedNodeArgeName.c_str()) + ); + } + else + { + return fusedNodeArgeName; + } + } } // namespace GraphKernelHelper } // namespace Dml \ No newline at end of file diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphKernelHelper.h b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphKernelHelper.h index 6f271612c6..63efb58300 100644 --- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphKernelHelper.h +++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/GraphKernelHelper.h @@ -63,6 +63,8 @@ namespace GraphKernelHelper _Out_ std::vector& dmlInputEdges, _Out_ std::vector& dmlOutputEdges, _Out_ std::vector& dmlIntermediateEdges); + + std::string GetFusedNodeArgNameMatchingGraph(const std::string& fusedNodeArgeName); } // namespace GraphKernelHelper } // namespace Dml \ No newline at end of file