From 7cb2c3f025d1c2ae894d90c16f3dd0c75480cac9 Mon Sep 17 00:00:00 2001 From: Dwayne Robinson Date: Tue, 30 Jun 2020 21:26:16 +0000 Subject: [PATCH] Merged PR 4852260: DML EP remove redundant rank checks for higher dimension support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove redundant checks in the DML EP which should instead rely on DML's validation. At least one of these checks wrongly prevents legitimate execution (5D Concat is supported in DML, but the DML EP blocks it 🤦‍♀️). Note this is a small aspect of the larger work (not sufficient to make the models fully work) that I thought I'd flush now while I had the change ready anyway due to investigation. Related work items: #23232293, #25707941 --- .../dml/DmlExecutionProvider/src/ExecutionProvider.cpp | 3 ++- .../src/Operators/DmlOperatorConcat.cpp | 2 +- .../src/Operators/DmlOperatorExpand.cpp | 8 ++++---- .../src/Operators/DmlOperatorGather.cpp | 1 - .../src/Operators/DmlOperatorReduce.cpp | 3 ++- .../providers/dml/DmlExecutionProvider/src/TensorDesc.h | 1 - .../dml/OperatorAuthorHelper/OperatorHelper.cpp | 9 ++------- 7 files changed, 11 insertions(+), 16 deletions(-) diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionProvider.cpp b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionProvider.cpp index 66e2f1ba06..011cdcbe24 100644 --- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionProvider.cpp +++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionProvider.cpp @@ -376,8 +376,9 @@ namespace Dml { assert(!m_closed); + const size_t sourceSizeInBytes = ComputeByteSizeFromTensor(*src); const size_t dataSizeInBytes = ComputeByteSizeFromTensor(*dst); - THROW_HR_IF(E_INVALIDARG, dataSizeInBytes != ComputeByteSizeFromTensor(*src)); // Tensors must be the same size + THROW_HR_IF(E_INVALIDARG, dataSizeInBytes != sourceSizeInBytes); // Tensors must be the same size if (dataSizeInBytes == 0) { diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorConcat.cpp b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorConcat.cpp index ce5c35935c..693c387d8c 100644 --- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorConcat.cpp +++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorConcat.cpp @@ -42,7 +42,7 @@ public: for (size_t i = 0; i < m_inputTensorDescs.size(); i++) { // DML doesn't support empty tensors for concat, so we ignore them - if (!OperatorHelper::ContainsEmptyDimensions(m_inputTensorDescs[i].GetDmlSizes())) + if (!OperatorHelper::ContainsEmptyDimensions(m_inputTensorDescs[i].GetSizes())) { inputDescs.push_back(m_inputTensorDescs[i].GetDmlDesc()); } diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorExpand.cpp b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorExpand.cpp index bf886fbd4f..a9f0213337 100644 --- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorExpand.cpp +++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorExpand.cpp @@ -25,8 +25,8 @@ public: TensorDesc inputTensorDesc = TensorDesc( kernelCreationContext.GetInputEdgeDescription(0).tensorDataType, - m_outputTensorDescs[0].GetDmlSizes(), - m_inputTensorDescs[0].GetDmlSizes(), + m_outputTensorDescs[0].GetSizes(), + m_inputTensorDescs[0].GetSizes(), TensorAxis::DoNotCoerce, TensorAxis::W, TensorAxis::RightAligned, @@ -36,8 +36,8 @@ public: TensorDesc outputTensorDesc = TensorDesc( kernelCreationContext.GetOutputEdgeDescription(0).tensorDataType, - m_outputTensorDescs[0].GetDmlSizes(), - m_outputTensorDescs[0].GetDmlSizes(), + m_outputTensorDescs[0].GetSizes(), + m_outputTensorDescs[0].GetSizes(), TensorAxis::DoNotCoerce, TensorAxis::W, TensorAxis::RightAligned, diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorGather.cpp b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorGather.cpp index 6850e71ab5..ec70544657 100644 --- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorGather.cpp +++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorGather.cpp @@ -27,7 +27,6 @@ public: auto outputTensorShapeDescription = kernelCreationContext.GetTensorShapeDescription(); std::vector dataDimensions = outputTensorShapeDescription.GetInputTensorShape(0); std::vector indicesDimensions = outputTensorShapeDescription.GetInputTensorShape(1); - ML_CHECK_VALID_ARGUMENT(dataDimensions.size() <= OperatorHelper::NchwDimensionCount); uint32_t dmlAxis = GetDmlAdjustedAxis(m_axis, kernelCreationContext, m_inputTensorDescs.front().GetDimensionCount()); DML_GATHER_OPERATOR_DESC operatorDesc = {}; diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorReduce.cpp b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorReduce.cpp index 7814d91e15..128d4308fe 100644 --- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorReduce.cpp +++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorReduce.cpp @@ -24,9 +24,10 @@ public: std::vector dmlAxes; std::vector reducedDims = kernelInfo.GetTensorShapeDescription().GetInputTensorShape(0); - int dimOffset = gsl::narrow_cast(OperatorHelper::NchwDimensionCount - reducedDims.size()); + int dimOffset = gsl::narrow_cast(m_inputTensorDescs[0].GetDimensionCount() - reducedDims.size()); for (auto& dim : m_axes) { + assert(dim < reducedDims.size()); // ReduceHelperBase already validated this. reducedDims[dim] = 1; dmlAxes.push_back(static_cast(dim + dimOffset)); } diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/TensorDesc.h b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/TensorDesc.h index 472984fdf5..ea1af04ad6 100644 --- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/TensorDesc.h +++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/TensorDesc.h @@ -36,7 +36,6 @@ namespace Dml inline DML_TENSOR_DATA_TYPE GetDmlDataType() const { return m_bufferTensorDesc.DataType; } inline MLOperatorTensorDataType GetMlOperatorDataType() const { return m_mlOperatorTensorDataType; } - inline gsl::span GetDmlSizes() const { return m_sizes; } void ForceUnsignedDataType(); void Remap64bitDmlDataTypeTo32bit(); bool WasRemapped64bitTo32bit() const; diff --git a/onnxruntime/core/providers/dml/OperatorAuthorHelper/OperatorHelper.cpp b/onnxruntime/core/providers/dml/OperatorAuthorHelper/OperatorHelper.cpp index 303a334841..5867c68a68 100644 --- a/onnxruntime/core/providers/dml/OperatorAuthorHelper/OperatorHelper.cpp +++ b/onnxruntime/core/providers/dml/OperatorAuthorHelper/OperatorHelper.cpp @@ -703,7 +703,7 @@ namespace OperatorHelper ML_CHECK_VALID_ARGUMENT(inputDimensions.size() >= 1); ML_CHECK_VALID_ARGUMENT(indicesDimensions.size() >= 0); int outDimCount = gsl::narrow_cast(inputDimensions.size() + indicesDimensions.size() - 1); - ML_CHECK_VALID_ARGUMENT(outDimCount >= 0 && outDimCount <= NchwDimensionCount); + ML_CHECK_VALID_ARGUMENT(outDimCount >= 0); std::vector outputDimensions(outDimCount, 1); @@ -746,7 +746,7 @@ namespace OperatorHelper const uint32_t numberOfOutputDimensionsFromInput = static_cast(inputDimensions.size()) - numberOfCoordinatesPerIndex; const uint32_t numberOfOutputDimensionsFromIndices = static_cast(indicesDimensions.size()) - 1; // Strip off last dimension. uint32_t outputDimensionCount = gsl::narrow_cast(numberOfOutputDimensionsFromIndices + numberOfOutputDimensionsFromInput); - ML_CHECK_VALID_ARGUMENT(outputDimensionCount > 0 && outputDimensionCount <= NchwDimensionCount); + ML_CHECK_VALID_ARGUMENT(outputDimensionCount > 0); // Form the full expected size by concatenating the prefix part of the indices tensor shape // with the suffix of the input tensor shape. @@ -812,8 +812,6 @@ namespace OperatorHelper // Dim Offset : 1 std::vector reducedDims = shapeInfo.GetInputTensorShape(0); - ML_CHECK_VALID_ARGUMENT(reducedDims.size() <= NchwDimensionCount); - std::vector reduced(reducedDims.size(), false); for (auto& dim : m_axes) @@ -847,8 +845,6 @@ namespace OperatorHelper void ReduceHelperBase::AdjustAxesAndOutputShape(const std::vector& inputShape) { - ML_CHECK_VALID_ARGUMENT(inputShape.size() <= NchwDimensionCount); - // If axes is not specified, reduce over all the dimensions if (m_axes.empty()) { @@ -1001,7 +997,6 @@ namespace OperatorHelper std::vector ConcatHelper::GetOutputShapes(const MLShapeInferenceContext& shapeInfo) const { auto outputShape = shapeInfo.GetInputTensorShape(0); - ML_CHECK_VALID_ARGUMENT(outputShape.size() <= NcdhwDimensionCount); uint32_t inputCount = shapeInfo.GetInputCount();