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
This commit is contained in:
Jeff Bloomfield 2020-10-26 22:16:22 +00:00
parent 49ec73e939
commit e380fd3c6b
3 changed files with 49 additions and 40 deletions

View file

@ -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());

View file

@ -109,7 +109,7 @@ namespace GraphKernelHelper
const std::unordered_map<std::string, onnx::TensorProto>& 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<const onnx::TensorProto*, uint32_t> 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<uint32_t>(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

View file

@ -63,6 +63,8 @@ namespace GraphKernelHelper
_Out_ std::vector<DML_GRAPH_EDGE_DESC>& dmlInputEdges,
_Out_ std::vector<DML_GRAPH_EDGE_DESC>& dmlOutputEdges,
_Out_ std::vector<DML_GRAPH_EDGE_DESC>& dmlIntermediateEdges);
std::string GetFusedNodeArgNameMatchingGraph(const std::string& fusedNodeArgeName);
} // namespace GraphKernelHelper
} // namespace Dml