From 50dbd62beadafbcd9687488c3e5ea0b52cc79afc Mon Sep 17 00:00:00 2001
From: Guoyu Wang <wanggy@outlook.com>
Date: Fri, 29 Jan 2021 22:43:13 -0800
Subject: [PATCH 1/4] [CoreML EP] Add support of Conv operator

---
 onnxruntime/core/providers/common.h           |   3 +-
 .../builders/impl/activation_op_builder.cc    |   6 +-
 .../coreml/builders/impl/base_op_builder.cc   |  14 +-
 .../coreml/builders/impl/base_op_builder.h    |   2 +
 .../coreml/builders/impl/binary_op_builder.cc |   3 +-
 .../coreml/builders/impl/conv_op_builder.cc   | 144 ++++++++++++++++++
 .../builders/impl/transpose_op_builder.cc     |   3 +-
 .../coreml/builders/op_builder_factory.cc     |   4 +
 .../coreml/builders/op_builder_factory.h      |   1 +
 .../nnapi_lib/NeuralNetworksWrapper.cc        |   3 +-
 .../core/providers/shared/utils/utils.cc      |   7 +
 .../core/providers/shared/utils/utils.h       |   7 +-
 .../test/providers/cpu/nn/conv_op_test.cc     |  12 +-
 13 files changed, 185 insertions(+), 24 deletions(-)
 create mode 100644 onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc

diff --git a/onnxruntime/core/providers/common.h b/onnxruntime/core/providers/common.h
index 17dfb39a3f883..3688133c6d8e3 100644
--- a/onnxruntime/core/providers/common.h
+++ b/onnxruntime/core/providers/common.h
@@ -140,9 +140,10 @@ inline bool Contains(const Map& map, const Key& key) {
   return map.find(key) != map.end();
 }
 
+// Note: This helper function will not have overflow protection
 template <template <typename...> class Container, typename T>
 T Product(const Container<T>& c) {
-  return static_cast<T>(accumulate(c.cbegin(), c.cend(), 1, std::multiplies<T>()));
+  return accumulate(c.cbegin(), c.cend(), 1, std::multiplies<T>());
 }
 
 }  // namespace onnxruntime
diff --git a/onnxruntime/core/providers/coreml/builders/impl/activation_op_builder.cc b/onnxruntime/core/providers/coreml/builders/impl/activation_op_builder.cc
index a3349486f073e..0b35c554392ea 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/activation_op_builder.cc
+++ b/onnxruntime/core/providers/coreml/builders/impl/activation_op_builder.cc
@@ -25,11 +25,9 @@ class ActivationOpBuilder : public BaseOpBuilder {
 Status ActivationOpBuilder::AddToModelBuilderImpl(ModelBuilder& model_builder,
                                                   const Node& node,
                                                   const logging::Logger& /* logger */) const {
-  const auto& op_type(node.OpType());
-
-  std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> layer = std::make_unique<COREML_SPEC::NeuralNetworkLayer>();
-  layer->set_name(node.Name());
+  std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> layer = CreateNNLayer(node);
 
+  const auto& op_type(node.OpType());
   if (op_type == "Sigmoid") {
     layer->mutable_activation()->mutable_sigmoid();
   } else if (op_type == "Tanh") {
diff --git a/onnxruntime/core/providers/coreml/builders/impl/base_op_builder.cc b/onnxruntime/core/providers/coreml/builders/impl/base_op_builder.cc
index 4679676a15d6b..9d2991dee1415 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/base_op_builder.cc
+++ b/onnxruntime/core/providers/coreml/builders/impl/base_op_builder.cc
@@ -48,6 +48,13 @@ Status BaseOpBuilder::AddToModelBuilder(ModelBuilder& model_builder, const Node&
   return Status::OK();
 }
 
+/* static */ std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> BaseOpBuilder::CreateNNLayer(const Node& node) {
+  std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> layer =
+      onnxruntime::make_unique<COREML_SPEC::NeuralNetworkLayer>();
+  layer->set_name(node.Name());
+  return layer;
+}
+
 // Operator support related
 
 bool BaseOpBuilder::IsOpSupported(const InitializedTensorSet& initializers, const Node& node,
@@ -66,12 +73,7 @@ bool BaseOpBuilder::IsOpSupported(const InitializedTensorSet& initializers, cons
 }
 
 bool BaseOpBuilder::HasSupportedInputs(const Node& node, const logging::Logger& logger) const {
-  std::string node_name = "Node [";
-  node_name += node.Name();
-  node_name += "] type [";
-  node_name += node.OpType();
-  node_name += "]";
-
+  const auto node_name = MakeString("Node [", node.Name(), "] type [", node.OpType(), "]");
   for (const auto* input : node.InputDefs()) {
     if (!IsInputSupported(*input, node_name, logger)) {
       return false;
diff --git a/onnxruntime/core/providers/coreml/builders/impl/base_op_builder.h b/onnxruntime/core/providers/coreml/builders/impl/base_op_builder.h
index 6b1cd1778029c..ce9d2dfbb26b6 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/base_op_builder.h
+++ b/onnxruntime/core/providers/coreml/builders/impl/base_op_builder.h
@@ -24,6 +24,8 @@ class BaseOpBuilder : public IOpBuilder {
   virtual Status AddToModelBuilderImpl(ModelBuilder& model_builder, const Node& node,
                                        const logging::Logger& logger) const ORT_MUST_USE_RESULT = 0;
 
+  static std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> CreateNNLayer(const Node& node);
+
   // Operator support related
  public:
   bool IsOpSupported(const InitializedTensorSet& initializers, const Node& node,
diff --git a/onnxruntime/core/providers/coreml/builders/impl/binary_op_builder.cc b/onnxruntime/core/providers/coreml/builders/impl/binary_op_builder.cc
index cedeab42c2305..b9dd9cb5b1575 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/binary_op_builder.cc
+++ b/onnxruntime/core/providers/coreml/builders/impl/binary_op_builder.cc
@@ -30,8 +30,7 @@ Status BinaryOpBuilder::AddToModelBuilderImpl(ModelBuilder& model_builder, const
   const auto& op_type(node.OpType());
   const auto input_defs(node.InputDefs());
 
-  std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> layer = std::make_unique<COREML_SPEC::NeuralNetworkLayer>();
-  layer->set_name(node.Name());
+  std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> layer = CreateNNLayer(node);
 
   if (op_type == "Add") {
     layer->mutable_add();
diff --git a/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc b/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
new file mode 100644
index 0000000000000..56d16aac33296
--- /dev/null
+++ b/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
@@ -0,0 +1,144 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+#include "core/providers/common.h"
+#include "core/providers/shared/utils/utils.h"
+#include "core/providers/coreml/builders/model_builder.h"
+#include "core/providers/coreml/builders/op_builder_factory.h"
+
+#include "base_op_builder.h"
+#include "builder_utils.h"
+
+namespace onnxruntime {
+namespace coreml {
+
+class ConvOpBuilder : public BaseOpBuilder {
+  // Add operator related
+ public:
+  void AddInitializersToSkip(ModelBuilder& model_builder, const Node& node) const override;
+
+ private:
+  Status AddToModelBuilderImpl(ModelBuilder& model_builder, const Node& node,
+                               const logging::Logger& logger) const override ORT_MUST_USE_RESULT;
+
+  // Operator support related
+ private:
+  bool IsOpSupportedImpl(const InitializedTensorSet& /* initializers */, const Node& /* node */,
+                         const logging::Logger& /* logger */) const override;
+};
+
+// Add operator related
+
+void ConvOpBuilder::AddInitializersToSkip(ModelBuilder& model_builder, const Node& node) const {
+  const auto input_defs = node.InputDefs();
+
+  // skip the weight and bias (if has it) for conv as we will directly set those as part of the NN layer
+  model_builder.AddInitializerToSkip(input_defs[1]->Name());  // w
+
+  if (input_defs.size() > 2) {
+    model_builder.AddInitializerToSkip(input_defs[2]->Name());  // b
+  }
+}
+
+Status ConvOpBuilder::AddToModelBuilderImpl(ModelBuilder& model_builder, const Node& node,
+                                            const logging::Logger& /* logger */) const {
+  std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> layer = CreateNNLayer(node);
+
+  const auto input_defs = node.InputDefs();
+
+  const auto& weight_tensor = *model_builder.GetInitializerTensors().at(input_defs[1]->Name());
+  const auto& weight_shape = weight_tensor.dims();
+
+  NodeAttrHelper helper(node);
+  const auto strides = helper.Get("strides", std::vector<int64_t>{1, 1});
+  const auto onnx_pads = helper.Get("pads", std::vector<int64_t>{0, 0, 0, 0});
+  const auto dilations = helper.Get("dilations", std::vector<int64_t>{1, 1});
+  const auto group = helper.Get("group", static_cast<int64_t>(1));
+
+  auto* coreml_conv = layer->mutable_convolution();
+
+  coreml_conv->set_outputchannels(weight_shape[0]);  // M
+  coreml_conv->set_kernelchannels(weight_shape[1]);  // C/Group
+  coreml_conv->add_kernelsize(weight_shape[2]);      // H
+  coreml_conv->add_kernelsize(weight_shape[3]);      // W
+  coreml_conv->set_ngroups(group);
+  *coreml_conv->mutable_stride() = {strides.cbegin(), strides.cend()};
+  *coreml_conv->mutable_dilationfactor() = {dilations.cbegin(), dilations.cend()};
+
+  coreml_conv->set_isdeconvolution(false);
+
+  // Padding
+  const auto auto_pad_type = StringToAutoPadType(helper.Get("auto_pad", "NOTSET"));
+  if (AutoPadType::SAME_UPPER == auto_pad_type || AutoPadType::SAME_LOWER == auto_pad_type) {
+    auto* padding_type = coreml_conv->mutable_same();
+    if (AutoPadType::SAME_LOWER == auto_pad_type) {  // default is SAME_UPPER
+      padding_type->set_asymmetrymode(COREML_SPEC::SamePadding_SamePaddingMode_TOP_LEFT_HEAVY);
+    }
+  } else {
+    auto* padding_type = coreml_conv->mutable_valid();
+    if (AutoPadType::NOTSET == auto_pad_type) {
+      // NOTSET is adding the explicit padding to the ValidPadding.paddingAmounts
+      auto* heightBorder = padding_type->mutable_paddingamounts()->add_borderamounts();
+      heightBorder->set_startedgesize(onnx_pads[0]);
+      heightBorder->set_endedgesize(onnx_pads[2]);
+      auto* widthBorder = padding_type->mutable_paddingamounts()->add_borderamounts();
+      widthBorder->set_startedgesize(onnx_pads[1]);
+      widthBorder->set_endedgesize(onnx_pads[3]);
+    }
+  }
+
+  // Add weight
+  CreateCoreMLWeight(*coreml_conv->mutable_weights(), weight_tensor);
+
+  // Add bias if present
+  if (input_defs.size() > 2) {
+    coreml_conv->set_hasbias(true);
+    const auto& bias_tensor = *model_builder.GetInitializerTensors().at(input_defs[2]->Name());
+    CreateCoreMLWeight(*coreml_conv->mutable_bias(), bias_tensor);
+  }
+
+  *layer->mutable_input()->Add() = node.InputDefs()[0]->Name();
+  *layer->mutable_output()->Add() = node.OutputDefs()[0]->Name();
+
+  model_builder.AddLayer(layer.release());
+  return Status::OK();
+}
+
+// Operator support related
+
+bool ConvOpBuilder::IsOpSupportedImpl(const InitializedTensorSet& initializers, const Node& node,
+                                      const logging::Logger& logger) const {
+  const auto& name = node.Name();
+  const auto input_defs = node.InputDefs();
+
+  const auto& weight_name = input_defs[1]->Name();
+  if (Contains(initializers, weight_name)) {
+    const auto& tensor = *initializers.at(weight_name);
+    if (tensor.dims().size() != 4) {
+      LOGS(logger, VERBOSE) << "Conv [" << name << "] dimension: " << tensor.dims().size()
+                            << " Only conv 2d is supported.";
+      return false;
+    }
+  } else {
+    LOGS(logger, VERBOSE) << "The weight of Conv [" << name << "] must be known";
+    return false;
+  }
+
+  if (input_defs.size() > 2) {
+    const auto& bias_name = input_defs[2]->Name();
+    if (!Contains(initializers, bias_name)) {
+      LOGS(logger, VERBOSE) << "The bias of Conv [" << name << "] must be known";
+      return false;
+    }
+  }
+
+  return true;
+}
+
+void CreateConvOpBuilder(const std::string& op_type, OpBuilderRegistrations& op_registrations) {
+  op_registrations.builders.push_back(onnxruntime::make_unique<ConvOpBuilder>());
+  op_registrations.op_builder_map.emplace(op_type, op_registrations.builders.back().get());
+}
+
+}  // namespace coreml
+}  // namespace onnxruntime
diff --git a/onnxruntime/core/providers/coreml/builders/impl/transpose_op_builder.cc b/onnxruntime/core/providers/coreml/builders/impl/transpose_op_builder.cc
index 73cf7011a2a6d..300a786e90ddb 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/transpose_op_builder.cc
+++ b/onnxruntime/core/providers/coreml/builders/impl/transpose_op_builder.cc
@@ -23,8 +23,7 @@ class TransposeOpBuilder : public BaseOpBuilder {
 Status TransposeOpBuilder::AddToModelBuilderImpl(ModelBuilder& model_builder,
                                                  const Node& node,
                                                  const logging::Logger& /* logger */) const {
-  std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> layer = std::make_unique<COREML_SPEC::NeuralNetworkLayer>();
-  layer->set_name(node.Name());
+  std::unique_ptr<COREML_SPEC::NeuralNetworkLayer> layer = CreateNNLayer(node);
 
   NodeAttrHelper helper(node);
   std::vector<int64_t> perm = helper.Get("perm", std::vector<int64_t>());
diff --git a/onnxruntime/core/providers/coreml/builders/op_builder_factory.cc b/onnxruntime/core/providers/coreml/builders/op_builder_factory.cc
index 694a4a88b73f5..6596282e3c9fe 100644
--- a/onnxruntime/core/providers/coreml/builders/op_builder_factory.cc
+++ b/onnxruntime/core/providers/coreml/builders/op_builder_factory.cc
@@ -29,6 +29,10 @@ static OpBuilderRegistrations CreateOpBuilderRegistrations() {
     CreateTransposeOpBuilder("Transpose", op_registrations);
   }
 
+  {  // Conv
+    CreateConvOpBuilder("Conv", op_registrations);
+  }
+
   return op_registrations;
 }
 
diff --git a/onnxruntime/core/providers/coreml/builders/op_builder_factory.h b/onnxruntime/core/providers/coreml/builders/op_builder_factory.h
index fc4cc1bbd7cc5..553c1bc1bd2f8 100644
--- a/onnxruntime/core/providers/coreml/builders/op_builder_factory.h
+++ b/onnxruntime/core/providers/coreml/builders/op_builder_factory.h
@@ -20,6 +20,7 @@ const std::unordered_map<std::string, const IOpBuilder*>& GetOpBuilders();
 
 void CreateBinaryOpBuilder(const std::string& op_type, OpBuilderRegistrations& op_registrations);
 void CreateTransposeOpBuilder(const std::string& op_type, OpBuilderRegistrations& op_registrations);
+void CreateConvOpBuilder(const std::string& op_type, OpBuilderRegistrations& op_registrations);
 
 void CreateActivationOpBuilder(const std::string& op_type, OpBuilderRegistrations& op_registrations);
 
diff --git a/onnxruntime/core/providers/nnapi/nnapi_builtin/nnapi_lib/NeuralNetworksWrapper.cc b/onnxruntime/core/providers/nnapi/nnapi_builtin/nnapi_lib/NeuralNetworksWrapper.cc
index ddbcd9edd0d24..491e74417bf06 100644
--- a/onnxruntime/core/providers/nnapi/nnapi_builtin/nnapi_lib/NeuralNetworksWrapper.cc
+++ b/onnxruntime/core/providers/nnapi/nnapi_builtin/nnapi_lib/NeuralNetworksWrapper.cc
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 #include <core/common/common.h>
+#include <core/common/safeint.h>
 
 #include "NeuralNetworksWrapper.h"
 
@@ -100,7 +101,7 @@ size_t OperandType::GetElementByteSize() const {
 
 size_t OperandType::GetOperandBlobByteSize() const {
   size_t num_elements = std::accumulate(dimensions.begin(), dimensions.end(), 1, std::multiplies<size_t>());
-  return num_elements * GetElementByteSize();
+  return SafeInt<size_t>(num_elements * GetElementByteSize());
 }
 
 void OperandType::SetDimensions(const std::vector<uint32_t>& d) {
diff --git a/onnxruntime/core/providers/shared/utils/utils.cc b/onnxruntime/core/providers/shared/utils/utils.cc
index aee3b3d2596f2..f83e951f7a33e 100644
--- a/onnxruntime/core/providers/shared/utils/utils.cc
+++ b/onnxruntime/core/providers/shared/utils/utils.cc
@@ -28,6 +28,13 @@ int32_t NodeAttrHelper::Get(const std::string& key, int32_t def_val) const {
   return SafeInt<int32_t>(node_attributes_.at(key).i());
 }
 
+int64_t NodeAttrHelper::Get(const std::string& key, int64_t def_val) const {
+  if (!HasAttr(key))
+    return def_val;
+
+  return node_attributes_.at(key).i();
+}
+
 std::string NodeAttrHelper::Get(const std::string& key, const std::string& def_val) const {
   if (!HasAttr(key))
     return def_val;
diff --git a/onnxruntime/core/providers/shared/utils/utils.h b/onnxruntime/core/providers/shared/utils/utils.h
index e023455cb3f46..c87c693144b36 100644
--- a/onnxruntime/core/providers/shared/utils/utils.h
+++ b/onnxruntime/core/providers/shared/utils/utils.h
@@ -21,13 +21,16 @@ class NodeAttrHelper {
   NodeAttrHelper(const onnxruntime::Node& node);
 
   float Get(const std::string& key, float def_val) const;
-  int32_t Get(const std::string& key, int32_t def_val) const;
+
+  int64_t Get(const std::string& key, int64_t def_val) const;
+
   std::string Get(const std::string& key, const std::string& def_val) const;
 
   std::vector<int64_t> Get(const std::string& key, const std::vector<int64_t>& def_val) const;
   std::vector<float> Get(const std::string& key, const std::vector<float>& def_val) const;
 
-  // Convert the ints() of the attribute from int64_t to int32_t
+  // Convert the i() or ints() of the attribute from int64_t to int32_t
+  int32_t Get(const std::string& key, int32_t def_val) const;
   std::vector<int32_t> Get(const std::string& key, const std::vector<int32_t>& def_val) const;
 
   bool HasAttr(const std::string& key) const;
diff --git a/onnxruntime/test/providers/cpu/nn/conv_op_test.cc b/onnxruntime/test/providers/cpu/nn/conv_op_test.cc
index 4dc9f831bf2d4..104705487a9a8 100644
--- a/onnxruntime/test/providers/cpu/nn/conv_op_test.cc
+++ b/onnxruntime/test/providers/cpu/nn/conv_op_test.cc
@@ -204,7 +204,7 @@ TEST(ConvTest, Conv2D_1) {
 
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape);
 
-  // NNAPI EP requires weight to be an initializer
+  // NNAPI/CoreML EP requires weight to be an initializer
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape, true);
 }
 
@@ -297,7 +297,7 @@ TEST(ConvTest, Conv2D_2) {
                         -0.18779225647449493f};
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape);
 
-  // NNAPI EP requires weight to be an initializer
+  // NNAPI/CoreML EP requires weight to be an initializer
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape, true);
 }
 
@@ -323,7 +323,7 @@ TEST(ConvTest, Conv2D_Bias_1) {
 
   TestConvOp(attrs, {X, W, B}, {X_shape, W_shape, B_shape}, expected_vals, Y_shape);
 
-  // NNAPI EP requires weight to be an initializer
+  // NNAPI/CoreML EP requires weight to be an initializer
   TestConvOp(attrs, {X, W, B}, {X_shape, W_shape, B_shape}, expected_vals, Y_shape, true);
 }
 
@@ -404,7 +404,7 @@ TEST(ConvTest, Conv2D_AutoPad1) {
                         12.0f, 15.0f, 15.0f, 15.0f, 8.0f};
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape);
 
-  // NNAPI EP requires weight to be an initializer
+  // NNAPI/CoreML EP requires weight to be an initializer
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape, true);
 }
 
@@ -438,7 +438,7 @@ TEST(ConvTest, Conv2D_AutoPad2) {
                         5.0f, 10.0f, 5.0f, 10.0f, 5.0f};
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape);
 
-  // NNAPI EP requires weight to be an initializer
+  // NNAPI/CoreML EP requires weight to be an initializer
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape, true);
 }
 
@@ -626,7 +626,7 @@ TEST(ConvTest, Conv2D_group) {
 
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape);
 
-  // NNAPI EP requires weight to be an initializer
+  // NNAPI/CoreML EP requires weight to be an initializer
   TestConvOp(attrs, {X, W}, {X_shape, W_shape}, expected_vals, Y_shape, true);
 }
 

From 3d2a9bd125486d5192966c1d63d2831a017f0b13 Mon Sep 17 00:00:00 2001
From: Guoyu Wang <wanggy@outlook.com>
Date: Fri, 29 Jan 2021 22:54:04 -0800
Subject: [PATCH 2/4] Ignore an corner case setting empty padding

---
 .../core/providers/coreml/builders/impl/conv_op_builder.cc      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc b/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
index 56d16aac33296..39c938e73b7d8 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
+++ b/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
@@ -76,7 +76,7 @@ Status ConvOpBuilder::AddToModelBuilderImpl(ModelBuilder& model_builder, const N
     }
   } else {
     auto* padding_type = coreml_conv->mutable_valid();
-    if (AutoPadType::NOTSET == auto_pad_type) {
+    if (AutoPadType::NOTSET == auto_pad_type && onnx_pads != std::vector<int64_t>{0, 0, 0, 0}) {
       // NOTSET is adding the explicit padding to the ValidPadding.paddingAmounts
       auto* heightBorder = padding_type->mutable_paddingamounts()->add_borderamounts();
       heightBorder->set_startedgesize(onnx_pads[0]);

From 94bc35cf1450577c6305a2c63785ba8b27e291bf Mon Sep 17 00:00:00 2001
From: Guoyu Wang <wanggy@outlook.com>
Date: Sat, 30 Jan 2021 20:51:22 -0800
Subject: [PATCH 3/4] Add handle autopadding

---
 .../coreml/builders/impl/builder_utils.cc     | 70 +++++++++++++++++++
 .../coreml/builders/impl/builder_utils.h      | 11 +++
 .../coreml/builders/impl/conv_op_builder.cc   | 14 +++-
 3 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/onnxruntime/core/providers/coreml/builders/impl/builder_utils.cc b/onnxruntime/core/providers/coreml/builders/impl/builder_utils.cc
index 7d1e6083f9a95..001cdeeaadd3f 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/builder_utils.cc
+++ b/onnxruntime/core/providers/coreml/builders/impl/builder_utils.cc
@@ -10,6 +10,76 @@
 namespace onnxruntime {
 namespace coreml {
 
+common::Status ComputeConvPads(const std::vector<int64_t> input_shape,
+                               const int64_t weight_size_y,
+                               const int64_t weight_size_x,
+                               const std::vector<int64_t>& onnx_pads,
+                               const std::vector<int64_t>& onnx_strides,
+                               const std::vector<int64_t>& onnx_dilations,
+                               AutoPadType auto_pad_type,
+                               std::vector<int64_t>& pads_out) {
+  const int64_t input_size_y = input_shape[2];
+  const int64_t input_size_x = input_shape[3];
+  const int64_t stride_y = onnx_strides[0];
+  const int64_t stride_x = onnx_strides[1];
+  const int64_t dilation_y = onnx_dilations[0];
+  const int64_t dilation_x = onnx_dilations[1];
+
+  int64_t padding_top = onnx_pads[0];
+  int64_t padding_bottom = onnx_pads[2];
+  int64_t padding_left = onnx_pads[1];
+  int64_t padding_right = onnx_pads[3];
+
+  ORT_RETURN_IF_ERROR(ComputePad(input_size_y,
+                                 stride_y, weight_size_y, dilation_y,
+                                 auto_pad_type,
+                                 padding_top, padding_bottom));
+  ORT_RETURN_IF_ERROR(ComputePad(input_size_x,
+                                 stride_x, weight_size_x, dilation_x,
+                                 auto_pad_type,
+                                 padding_left, padding_right));
+
+  pads_out = {padding_top, padding_left, padding_bottom, padding_right};
+
+  return Status::OK();
+}
+
+common::Status HandleAutoPad(const std::vector<int64_t> input_shape,
+                             const int64_t weight_size_y,
+                             const int64_t weight_size_x,
+                             const std::vector<int64_t>& onnx_pads,
+                             const std::vector<int64_t>& onnx_strides,
+                             const std::vector<int64_t>& onnx_dilations,
+                             AutoPadType auto_pad_type,
+                             AutoPadType& auto_pad_type_out) {
+  auto_pad_type_out = auto_pad_type;
+  if (auto_pad_type == AutoPadType::NOTSET && onnx_dilations == std::vector<int64_t>{1, 1}) {
+    {
+      std::vector<int64_t> same_upper_pads;
+      ORT_RETURN_IF_ERROR(ComputeConvPads(input_shape, weight_size_y, weight_size_x,
+                                          onnx_pads, onnx_strides, onnx_dilations,
+                                          AutoPadType::SAME_UPPER, same_upper_pads));
+      if (onnx_pads == same_upper_pads) {
+        auto_pad_type_out = AutoPadType::SAME_UPPER;
+        return Status::OK();
+      }
+    }
+
+    {
+      std::vector<int64_t> same_lower_pads;
+      ORT_RETURN_IF_ERROR(ComputeConvPads(input_shape, weight_size_y, weight_size_x,
+                                          onnx_pads, onnx_strides, onnx_dilations,
+                                          AutoPadType::SAME_UPPER, same_lower_pads));
+      if (onnx_pads == same_lower_pads) {
+        auto_pad_type_out = AutoPadType::SAME_LOWER;
+        return Status::OK();
+      }
+    }
+  }
+
+  return Status::OK();
+}
+
 common::Status CreateCoreMLWeight(CoreML::Specification::WeightParams& weight,
                                   const ONNX_NAMESPACE::TensorProto& tensor) {
   auto data_type = tensor.data_type();
diff --git a/onnxruntime/core/providers/coreml/builders/impl/builder_utils.h b/onnxruntime/core/providers/coreml/builders/impl/builder_utils.h
index 683a9cbb560ec..ef789489c6aa6 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/builder_utils.h
+++ b/onnxruntime/core/providers/coreml/builders/impl/builder_utils.h
@@ -18,6 +18,17 @@ class WeightParams;
 namespace onnxruntime {
 namespace coreml {
 
+// Try to see if we can map explicit padding to auto padding for Conv/Pool
+// Since usually use auto padding is more efficient
+common::Status HandleAutoPad(const std::vector<int64_t> input_shape,
+                             const int64_t weight_size_y,
+                             const int64_t weight_size_x,
+                             const std::vector<int64_t>& onnx_pads,
+                             const std::vector<int64_t>& onnx_strides,
+                             const std::vector<int64_t>& onnx_dilations,
+                             AutoPadType auto_pad_type,
+                             AutoPadType& auto_pad_type_out) ORT_MUST_USE_RESULT;
+
 // Copy an onnx initializer data to a coreml weight
 common::Status CreateCoreMLWeight(CoreML::Specification::WeightParams& weight,
                                   const ONNX_NAMESPACE::TensorProto& tensor);
diff --git a/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc b/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
index 39c938e73b7d8..718238f36de87 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
+++ b/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
@@ -3,6 +3,7 @@
 
 #include "core/providers/common.h"
 #include "core/providers/shared/utils/utils.h"
+#include "core/providers/coreml/builders/helper.h"
 #include "core/providers/coreml/builders/model_builder.h"
 #include "core/providers/coreml/builders/op_builder_factory.h"
 
@@ -67,8 +68,17 @@ Status ConvOpBuilder::AddToModelBuilderImpl(ModelBuilder& model_builder, const N
 
   coreml_conv->set_isdeconvolution(false);
 
-  // Padding
-  const auto auto_pad_type = StringToAutoPadType(helper.Get("auto_pad", "NOTSET"));
+  // Add Padding
+  // Usually using autopadding is more efficient than using explicit padding
+  // Try to see if we can map explicit padding to auto padding
+  std::vector<int64_t> input_shape;
+  ORT_RETURN_IF_ERROR(GetShape(*input_defs[0], input_shape));
+  AutoPadType auto_pad_type;
+  ORT_RETURN_IF_ERROR(HandleAutoPad(input_shape, weight_shape[2], weight_shape[3],
+                                    onnx_pads, strides, dilations,
+                                    StringToAutoPadType(helper.Get("auto_pad", "NOTSET")),
+                                    auto_pad_type));
+
   if (AutoPadType::SAME_UPPER == auto_pad_type || AutoPadType::SAME_LOWER == auto_pad_type) {
     auto* padding_type = coreml_conv->mutable_same();
     if (AutoPadType::SAME_LOWER == auto_pad_type) {  // default is SAME_UPPER

From 38aa271a23b7c1424ac5ea13dc049b6d018a3199 Mon Sep 17 00:00:00 2001
From: Guoyu Wang <wanggy@outlook.com>
Date: Wed, 3 Feb 2021 15:55:14 -0800
Subject: [PATCH 4/4] Addressed CR comments

---
 .../coreml/builders/impl/builder_utils.cc          |  2 +-
 .../coreml/builders/impl/conv_op_builder.cc        | 14 +++++++-------
 .../nnapi_lib/NeuralNetworksWrapper.cc             |  5 +++--
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/onnxruntime/core/providers/coreml/builders/impl/builder_utils.cc b/onnxruntime/core/providers/coreml/builders/impl/builder_utils.cc
index 001cdeeaadd3f..90455f72458c3 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/builder_utils.cc
+++ b/onnxruntime/core/providers/coreml/builders/impl/builder_utils.cc
@@ -69,7 +69,7 @@ common::Status HandleAutoPad(const std::vector<int64_t> input_shape,
       std::vector<int64_t> same_lower_pads;
       ORT_RETURN_IF_ERROR(ComputeConvPads(input_shape, weight_size_y, weight_size_x,
                                           onnx_pads, onnx_strides, onnx_dilations,
-                                          AutoPadType::SAME_UPPER, same_lower_pads));
+                                          AutoPadType::SAME_LOWER, same_lower_pads));
       if (onnx_pads == same_lower_pads) {
         auto_pad_type_out = AutoPadType::SAME_LOWER;
         return Status::OK();
diff --git a/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc b/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
index 718238f36de87..98fb0c3f2e63e 100644
--- a/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
+++ b/onnxruntime/core/providers/coreml/builders/impl/conv_op_builder.cc
@@ -88,12 +88,12 @@ Status ConvOpBuilder::AddToModelBuilderImpl(ModelBuilder& model_builder, const N
     auto* padding_type = coreml_conv->mutable_valid();
     if (AutoPadType::NOTSET == auto_pad_type && onnx_pads != std::vector<int64_t>{0, 0, 0, 0}) {
       // NOTSET is adding the explicit padding to the ValidPadding.paddingAmounts
-      auto* heightBorder = padding_type->mutable_paddingamounts()->add_borderamounts();
-      heightBorder->set_startedgesize(onnx_pads[0]);
-      heightBorder->set_endedgesize(onnx_pads[2]);
-      auto* widthBorder = padding_type->mutable_paddingamounts()->add_borderamounts();
-      widthBorder->set_startedgesize(onnx_pads[1]);
-      widthBorder->set_endedgesize(onnx_pads[3]);
+      auto* height_border = padding_type->mutable_paddingamounts()->add_borderamounts();
+      height_border->set_startedgesize(onnx_pads[0]);
+      height_border->set_endedgesize(onnx_pads[2]);
+      auto* width_border = padding_type->mutable_paddingamounts()->add_borderamounts();
+      width_border->set_startedgesize(onnx_pads[1]);
+      width_border->set_endedgesize(onnx_pads[3]);
     }
   }
 
@@ -137,7 +137,7 @@ bool ConvOpBuilder::IsOpSupportedImpl(const InitializedTensorSet& initializers,
   if (input_defs.size() > 2) {
     const auto& bias_name = input_defs[2]->Name();
     if (!Contains(initializers, bias_name)) {
-      LOGS(logger, VERBOSE) << "The bias of Conv [" << name << "] must be known";
+      LOGS(logger, VERBOSE) << "The bias of Conv [" << name << "] must be a constant initializer";
       return false;
     }
   }
diff --git a/onnxruntime/core/providers/nnapi/nnapi_builtin/nnapi_lib/NeuralNetworksWrapper.cc b/onnxruntime/core/providers/nnapi/nnapi_builtin/nnapi_lib/NeuralNetworksWrapper.cc
index 491e74417bf06..78b8f0f9b944d 100644
--- a/onnxruntime/core/providers/nnapi/nnapi_builtin/nnapi_lib/NeuralNetworksWrapper.cc
+++ b/onnxruntime/core/providers/nnapi/nnapi_builtin/nnapi_lib/NeuralNetworksWrapper.cc
@@ -100,8 +100,9 @@ size_t OperandType::GetElementByteSize() const {
 }
 
 size_t OperandType::GetOperandBlobByteSize() const {
-  size_t num_elements = std::accumulate(dimensions.begin(), dimensions.end(), 1, std::multiplies<size_t>());
-  return SafeInt<size_t>(num_elements * GetElementByteSize());
+  // use uin64_t even dimension is uint32_t to prevent overflow
+  uint64_t num_elements = std::accumulate(dimensions.begin(), dimensions.end(), 1, std::multiplies<uint64_t>());
+  return SafeInt<size_t>(num_elements) * GetElementByteSize();
 }
 
 void OperandType::SetDimensions(const std::vector<uint32_t>& d) {