lhutton1 commented on code in PR #13092:
URL: https://github.com/apache/tvm/pull/13092#discussion_r999558469


##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -64,14 +64,42 @@ def ConvertEquivalents() -> tvm.ir.IRModule:  # pylint: 
disable=invalid-name
     """Converts operations into a numerically equivalent form
     that can be understood by the NPU codegen.
 
-    Return
-    ------
+    Returns
+    -------
     Pass
         The module pass.
     """
     return _ethosn.ConvertEquivalents()
 
 
+def InlineNonComputeIntensivePartitions() -> tvm.ir.IRModule:  # pylint: 
disable=invalid-name
+    """This pass checks whether functions partitioned for the NPU are 
considered
+    non-compute intensive. If they are not, they will be unpartitioned and 
passed onto
+    other backends to consider.
+
+    A partitioned function is currently considered non-compute intensive if it 
contains
+    no multiply accumulate operations.
+
+    Returns
+    -------
+    Pass
+        The module pass.
+    """
+    return _ethosn.InlineNonComputeIntensivePartitions()
+
+
+def inline_non_compute_intensive_partitions() -> bool:

Review Comment:
   Yep that makes sense



##########
src/relay/backend/contrib/ethosn/inline_partitions.cc:
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/relay/backend/contrib/ethosn/inline_partitions.cc
+ * \brief A pass to inline NPU partitions that are not considered compute
+ * intensive.
+ */
+
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+
+#include "../../../transforms/compiler_function_utils.h"
+
+namespace tvm {
+namespace relay {
+namespace contrib {
+namespace ethosn {
+
+class IsComputeIntensivePartition : MixedModeVisitor {
+ public:
+  /*!
+   * \brief Check if the partitioned function is compute
+   * intensive. If it has not multiply-accumulate operations
+   * it is not considered compute intensive.
+   *
+   * \param expr The partitioned function to check.
+   */
+  bool CheckSubgraph(const Expr& expr) {
+    is_compute_intensive = false;
+    VisitExpr(expr);
+    return is_compute_intensive;
+  }
+
+  /*!
+   * \brief Visit the call nodes of a partitioned function
+   * and check if operators or composite functions make the
+   * partitioned function compute intensive.
+   *
+   * \param op The call node to check.
+   */
+  void VisitExpr_(const CallNode* op) override {
+    Call call = GetRef<Call>(op);
+    std::string op_name = "";
+    if (const auto* op = call->op.as<OpNode>()) {
+      op_name = op->name;
+    } else if (const auto* func = call->op.as<FunctionNode>()) {
+      op_name = func->GetAttr<String>(attr::kComposite, "").value();
+    }
+
+    if (op_name != "") {
+      if (compute_intensive_operators.find(op_name) != 
compute_intensive_operators.end()) {
+        is_compute_intensive = true;
+      }
+    }
+  }
+
+ private:
+  /*! \brief Whether or not the partitioned function is consdiered compute 
intensive. */
+  bool is_compute_intensive;
+  /*! \brief A set of operators considered compute intensive. */
+  const std::unordered_set<std::string> compute_intensive_operators{

Review Comment:
   I was a bit hesitant to make this pass more generic as different hardware 
might require a different heuristic all together (rather than just a different 
list of operators). That said, it does seem useful for the user to 
customise/tune the pass from TVMC if necessary, especially since the heuristic 
is not optimal. But yep, think this would be good as a followup



##########
src/relay/backend/contrib/ethosn/inline_partitions.cc:
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/relay/backend/contrib/ethosn/inline_partitions.cc
+ * \brief A pass to inline NPU partitions that are not considered compute
+ * intensive.
+ */
+
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+
+#include "../../../transforms/compiler_function_utils.h"
+
+namespace tvm {
+namespace relay {
+namespace contrib {
+namespace ethosn {
+
+class IsComputeIntensivePartition : MixedModeVisitor {
+ public:
+  /*!
+   * \brief Check if the partitioned function is compute
+   * intensive. If it has not multiply-accumulate operations
+   * it is not considered compute intensive.
+   *
+   * \param expr The partitioned function to check.
+   */
+  bool CheckSubgraph(const Expr& expr) {
+    is_compute_intensive = false;
+    VisitExpr(expr);
+    return is_compute_intensive;
+  }
+
+  /*!
+   * \brief Visit the call nodes of a partitioned function
+   * and check if operators or composite functions make the
+   * partitioned function compute intensive.
+   *
+   * \param op The call node to check.
+   */
+  void VisitExpr_(const CallNode* op) override {
+    Call call = GetRef<Call>(op);
+    std::string op_name = "";
+    if (const auto* op = call->op.as<OpNode>()) {
+      op_name = op->name;
+    } else if (const auto* func = call->op.as<FunctionNode>()) {
+      op_name = func->GetAttr<String>(attr::kComposite, "").value();
+    }
+
+    if (op_name != "") {
+      if (compute_intensive_operators.find(op_name) != 
compute_intensive_operators.end()) {

Review Comment:
   Yes currently that's possible. It's difficult to come up with a sensible 
limit here without being able to estimate the performance, perhaps if needed we 
can expose this option to the user in the future?



##########
src/relay/backend/contrib/ethosn/inline_partitions.cc:
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/relay/backend/contrib/ethosn/inline_partitions.cc
+ * \brief A pass to inline NPU partitions that are not considered compute
+ * intensive.
+ */
+
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+
+#include "../../../transforms/compiler_function_utils.h"
+
+namespace tvm {
+namespace relay {
+namespace contrib {
+namespace ethosn {
+
+class IsComputeIntensivePartition : MixedModeVisitor {
+ public:
+  /*!
+   * \brief Check if the partitioned function is compute
+   * intensive. If it has not multiply-accumulate operations
+   * it is not considered compute intensive.
+   *
+   * \param expr The partitioned function to check.
+   */
+  bool CheckSubgraph(const Expr& expr) {
+    is_compute_intensive = false;
+    VisitExpr(expr);
+    return is_compute_intensive;
+  }
+
+  /*!
+   * \brief Visit the call nodes of a partitioned function
+   * and check if operators or composite functions make the
+   * partitioned function compute intensive.
+   *
+   * \param op The call node to check.
+   */
+  void VisitExpr_(const CallNode* op) override {
+    Call call = GetRef<Call>(op);
+    std::string op_name = "";
+    if (const auto* op = call->op.as<OpNode>()) {
+      op_name = op->name;
+    } else if (const auto* func = call->op.as<FunctionNode>()) {
+      op_name = func->GetAttr<String>(attr::kComposite, "").value();
+    }
+
+    if (op_name != "") {
+      if (compute_intensive_operators.find(op_name) != 
compute_intensive_operators.end()) {
+        is_compute_intensive = true;
+      }
+    }
+  }
+
+ private:
+  /*! \brief Whether or not the partitioned function is consdiered compute 
intensive. */
+  bool is_compute_intensive;
+  /*! \brief A set of operators considered compute intensive. */
+  const std::unordered_set<std::string> compute_intensive_operators{
+      "ethos-n.qnn_add",        "ethos-n.qnn_conv2d",  
"ethos-n.qnn_conv2d_transpose",

Review Comment:
   Oops, good catch, thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to