You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2019/12/23 03:33:15 UTC

[GitHub] [incubator-tvm] zhiics opened a new pull request #4570: [relay] Relay annotation and partitioning for external compilers

zhiics opened a new pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570
 
 
   This is the annotation and partitioning part of #4258 
   
   It adds a separate build pipeline for users to have a Module->Module pass on a Relay program. Then users with take the annotated module and sent it to relay.build for compilation. An example could be like the following:
   
   ```python
   mod = relay.testing.mobilenet.get_workload(batch_size=1, dtype='float32')
   mod = relay.build_extern_compiler(mod, "dnnl")
   json, lib, param = relay.build(mod, target=target, params=params)
   ```
   
   Fellow-up PRs will work on tutorials to help developers add external codegen/runtime and create their own build pipeline.
   
   CC @tqchen @jroesch @icemelon9 @u99127 @masahi @u99127 @soiferj 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-568848069
 
 
   @masahi Sorry. I mistakenly changed the name of "ccompiler" directly. It therefore cannot find the registry and fallback to cpu. Now it is fixed. Please try it again.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361614728
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
 
 Review comment:
   Seems there are some repetitive GetRef<Op>(op_node) == Op::Get(...)
   I think it is better to use IsOp(call) defined in https://github.com/apache/incubator-tvm/blob/master/src/relay/backend/contrib/codegen_c/codegen_c.h#L195
   
   See the discussion in https://github.com/apache/incubator-tvm/pull/4482#discussion_r358093039

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
comaniac commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569198065
 
 
   > If BN is not decomposed, why am I not seeing dnnl_bn generated?
   It is not marked as external because `batch_norm` is not defined in python/tvm/relay/op/contrib/ccompiler/annotate_compiler.py
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569193712
 
 
   @zhiics When I run the mobilenet test and dump the annotated graph, batch norm is not converted to dnnl one. 
   ```
     %150 = fn (%dnnl_input75: Tensor[(1, 1024, 7, 7), float32], %dnnl_input76: Tensor[(1024, 1, 3, 3), float32], Compiler="dnnl", ExternalSymbol="dnnl_4", Primitive=1) -> Tensor[(1, 1024, 7, 7), float32] {
       nn.conv2d(%dnnl_input75, %dnnl_input76, padding=[1, 1], groups=1024, channels=1024, kernel_size=[3, 3]) /* ty=Tensor[(1, 1024, 7, 7), float32] */
     };
     %151 = %150(%149, %separable_conv_block_13_depthwise_conv1_weight) /* ty=Tensor[(1, 1024, 7, 7), float32] */;
     %152 = nn.batch_norm(%151, %separable_conv_block_13_bn1_gamma, %separable_conv_block_13_bn1_beta, %separable_conv_block_13_bn1_moving_mean, %separable_conv_block_13_bn1_moving_var) /* ty=(Tensor[(1, 1024, 7, 7), float32], Tensor[(1024), float32], Tensor[(1024), float32]) */;
     %153 = %152.0;
     %154 = fn (%dnnl_input77: Tensor[(1, 1024, 7, 7), float32], Compiler="dnnl", ExternalSymbol="dnnl_3", Primitive=1) -> Tensor[(1, 1024, 7, 7), float32] {
       nn.relu(%dnnl_input77) /* ty=Tensor[(1, 1024, 7, 7), float32] */
     };
   ```
   It is probably because batch norm is decomposed during SimplifyInference. Is batch norm supposed to be around during external codegen time? I think it should be. I don't hit "else if (IsOp(call, "nn.batch_norm")" condition [here](https://github.com/apache/incubator-tvm/blob/master/src/relay/backend/contrib/dnnl/codegen.cc#L73) during mobilenet test.
    
   I want to see an example of matching a pattern like Conv + BN + Relu and translate them to dnnl's fused op. It would be great if you could do that in this or future PR, otherwise I'll do it as a practice and send a PR :) 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-570838971
 
 
   can we merge this? @tqchen @jroesch @icemelon9

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361615655
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
+      }
+    }
+    return nullptr;
+  }
+
+  void MergeSubgraph(Subgraph* subgraph1, Subgraph* subgraph2) {
+    if (subgraph1 == subgraph2) {
+      return;
+    }
+
+    // Merge subgraph 2 to subgraph 1 and erase subgraph 2.
+    subgraph1->nodes.insert(subgraph2->nodes.begin(), subgraph2->nodes.end());
+    for (auto arg : subgraph2->args) {
+      subgraph1->args.push_back(arg);
+    }
+    this->subgraphs_.erase(subgraph2);
+  }
+
+  void AddToSubgraph(Subgraph* subgraph, const Expr expr) {
+    auto subgraph2 = GetSubgraph(expr);
+    if (subgraph2) {
+      MergeSubgraph(subgraph, subgraph2);
+    } else {
+      subgraph->nodes.insert(expr);
+    }
+  }
+
+  Expr VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      // Propogate subgraph to arguments
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (subgraph) {
+        for (auto arg : call->args) {
+          AddToSubgraph(subgraph, arg);
+        }
+      }
+      return ExprMutator::VisitExpr_(call);
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      // The annotation node is inserted on edge so it must have only one argument.
+      CHECK_EQ(call->args.size(), 1U);
+
+      // Traverse the rest graph.
+      auto input_expr = VisitExpr(call->args[0]);
+
+      // Replace the begin annotation with an external call input variable.
+      auto compiler_attrs = call->attrs.as<CompilerAttrs>();
+      auto var = VarNode::make(compiler_attrs->compiler + "_input" + std::to_string(var_id_++),
+                               input_expr->checked_type_);
+
+      // Find the corresponding subgraph and add the argument.
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (!subgraph) {
+        throw Error(RELAY_ERROR("Cannot find the corresponding subgraph for start annotation:\n"
+                                << AsText(GetRef<Call>(call), false)));
+      }
+      subgraph->args.push_back({var, input_expr});
+      return std::move(var);
+    } else {
+      CHECK(GetRef<Op>(op_node) == Op::Get("annotation.compiler_end"));
+      // The annotation node is inserted on edge so it must have only one argument.
+      CHECK_EQ(call->args.size(), 1U);
+
+      auto compiler_attrs = call->attrs.as<CompilerAttrs>();
+
+      // Check if the argument is already belonged to an exist subgraph
 
 Review comment:
   already belongs 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361615989
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
+      }
+    }
+    return nullptr;
+  }
+
+  void MergeSubgraph(Subgraph* subgraph1, Subgraph* subgraph2) {
+    if (subgraph1 == subgraph2) {
+      return;
+    }
+
+    // Merge subgraph 2 to subgraph 1 and erase subgraph 2.
+    subgraph1->nodes.insert(subgraph2->nodes.begin(), subgraph2->nodes.end());
+    for (auto arg : subgraph2->args) {
+      subgraph1->args.push_back(arg);
+    }
+    this->subgraphs_.erase(subgraph2);
+  }
+
+  void AddToSubgraph(Subgraph* subgraph, const Expr expr) {
+    auto subgraph2 = GetSubgraph(expr);
+    if (subgraph2) {
+      MergeSubgraph(subgraph, subgraph2);
+    } else {
+      subgraph->nodes.insert(expr);
+    }
+  }
+
+  Expr VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      // Propogate subgraph to arguments
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (subgraph) {
+        for (auto arg : call->args) {
+          AddToSubgraph(subgraph, arg);
+        }
+      }
+      return ExprMutator::VisitExpr_(call);
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      // The annotation node is inserted on edge so it must have only one argument.
+      CHECK_EQ(call->args.size(), 1U);
+
+      // Traverse the rest graph.
+      auto input_expr = VisitExpr(call->args[0]);
+
+      // Replace the begin annotation with an external call input variable.
+      auto compiler_attrs = call->attrs.as<CompilerAttrs>();
+      auto var = VarNode::make(compiler_attrs->compiler + "_input" + std::to_string(var_id_++),
+                               input_expr->checked_type_);
+
+      // Find the corresponding subgraph and add the argument.
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (!subgraph) {
+        throw Error(RELAY_ERROR("Cannot find the corresponding subgraph for start annotation:\n"
+                                << AsText(GetRef<Call>(call), false)));
+      }
+      subgraph->args.push_back({var, input_expr});
+      return std::move(var);
+    } else {
+      CHECK(GetRef<Op>(op_node) == Op::Get("annotation.compiler_end"));
+      // The annotation node is inserted on edge so it must have only one argument.
+      CHECK_EQ(call->args.size(), 1U);
+
+      auto compiler_attrs = call->attrs.as<CompilerAttrs>();
+
+      // Check if the argument is already belonged to an exist subgraph
+      auto subgraph = GetSubgraph(call->args[0]);
+      if (!subgraph) {
+        auto ret = this->subgraphs_.emplace(new Subgraph());
+        subgraph = *ret.first;
+        subgraph->nodes.insert(call->args[0]);
+        subgraph->id = this->subgraph_id_++;
+      }
+      subgraph->nodes.insert(GetRef<Call>(call));
+
+      // Traverse towarding to subgraph inputs.
+      auto input = VisitExpr(call->args[0]);
+      Array<Var> params;
+      Array<Expr> args;
+
+      // The subgraph may be merged so we need to update it again.
+      subgraph = GetSubgraph(GetRef<Call>(call));
 
 Review comment:
   may be better to add CHECK(subgraph)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361727260
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
 
 Review comment:
   Please see #4594, I sent a PR to refactor all places. Will update this PR, once it is merged.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569196731
 
 
   If BN is not decomposed, why am I not seeing dnnl_bn generated?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
comaniac commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569199848
 
 
   BTW, the `batch_norm` is also set to not decompose here: https://github.com/apache/incubator-tvm/blob/master/src/relay/pass/fuse_ops.cc#L243. This is not a general solution so we also plan to improve it later.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361727260
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
 
 Review comment:
   Please see #4594, I sent a PR to refactor all places. Will update this PR, once that one is merged.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361605864
 
 

 ##########
 File path: include/tvm/relay/attrs/annotation.h
 ##########
 @@ -57,6 +57,19 @@ struct CastHintAttrs : public tvm::AttrsNode<CastHintAttrs> {
   }
 };
 
+/*!
+ * \brief Options for the operators used to annotate a compiler.
+ */
+struct CompilerAttrs : public tvm::AttrsNode<CompilerAttrs> {
+  /*! \brief The 3rd party compiler for code generation. */
+  std::string compiler;
+
+  TVM_DECLARE_ATTRS(CompilerAttrs, "relay.attrs.CompilerAttrs") {
+    TVM_ATTR_FIELD(compiler)
+      .describe("The 3rd compiler used for code generation.");
 
 Review comment:
   A third party

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac edited a comment on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
comaniac edited a comment on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569198065
 
 
   > If BN is not decomposed, why am I not seeing dnnl_bn generated?
   
   It is not marked as external because `batch_norm` is always False in https://github.com/apache/incubator-tvm/pull/4570/files#diff-245f59aa1968d2efdb8357cfbdf149c9R40-R44
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361619208
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
+      }
+    }
+    return nullptr;
+  }
+
+  void MergeSubgraph(Subgraph* subgraph1, Subgraph* subgraph2) {
+    if (subgraph1 == subgraph2) {
+      return;
+    }
+
+    // Merge subgraph 2 to subgraph 1 and erase subgraph 2.
+    subgraph1->nodes.insert(subgraph2->nodes.begin(), subgraph2->nodes.end());
+    for (auto arg : subgraph2->args) {
+      subgraph1->args.push_back(arg);
+    }
+    this->subgraphs_.erase(subgraph2);
+  }
+
+  void AddToSubgraph(Subgraph* subgraph, const Expr expr) {
+    auto subgraph2 = GetSubgraph(expr);
+    if (subgraph2) {
+      MergeSubgraph(subgraph, subgraph2);
+    } else {
+      subgraph->nodes.insert(expr);
+    }
+  }
+
+  Expr VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      // Propogate subgraph to arguments
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (subgraph) {
+        for (auto arg : call->args) {
+          AddToSubgraph(subgraph, arg);
+        }
+      }
+      return ExprMutator::VisitExpr_(call);
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
 
 Review comment:
   better to use IsOp()

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-573520559
 
 
   As discussed in https://discuss.tvm.ai/t/rfc-naming-and-api-signature-for-external-compilation/5292, let's remove the annotation template for now. Instead, we only focus on partitioning and expect developers to annotate the program first.
   
   Followup PRs will be sent to help annotation which may need more discussion and thoughts on how to combine together with OpStrategy.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361598019
 
 

 ##########
 File path: tests/python/relay/test_pass_partition_graph.py
 ##########
 @@ -0,0 +1,391 @@
+# 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.
+"""Unit tests for graph partitioning."""
+import os
+import sys
+import numpy as np
+import pytest
+
+import tvm
+import tvm.relay.testing
+import tvm.relay.transform
+from tvm import relay
+from tvm.contrib import util
+from tvm.relay.annotation import compiler_begin, compiler_end
+from tvm.relay.expr_functor import ExprMutator
+
+
+class CcompilerAnnotator(ExprMutator):
+    """
+    A simple annotator that creates the following program:
+           |
+      -- begin --
+           |
+          add
+           |
+        subtract
+           |
+        multiply
+           |
+       -- end --
+           |
+    """
+
+    def __init__(self):
+        super(CcompilerAnnotator, self).__init__()
+        self.in_compiler = 0
+
+    def visit_call(self, call):
+        if call.op.name == "add":  # Annotate begin at args
+            if self.in_compiler == 1:
+                lhs = compiler_begin(super().visit(call.args[0]), "ccompiler")
+                rhs = compiler_begin(super().visit(call.args[1]), "ccompiler")
+                op = relay.add(lhs, rhs)
+                self.in_compiler = 2
+                return op
+        elif call.op.name == "subtract":
+            if self.in_compiler == 1:
+                lhs = super().visit(call.args[0])
+                rhs = super().visit(call.args[1])
+                if isinstance(lhs, relay.expr.Var):
+                    lhs = compiler_begin(lhs, "ccompiler")
+                if isinstance(rhs, relay.expr.Var):
+                    rhs = compiler_begin(rhs, "ccompiler")
+                return relay.subtract(lhs, rhs)
+        elif call.op.name == "multiply":  # Annotate end at output
+            self.in_compiler = 1
+            lhs = super().visit(call.args[0])
+            rhs = super().visit(call.args[1])
+            if isinstance(lhs, relay.expr.Var):
+                lhs = compiler_begin(lhs, "ccompiler")
+            if isinstance(rhs, relay.expr.Var):
+                rhs = compiler_begin(rhs, "ccompiler")
+            op = relay.multiply(lhs, rhs)
+            if self.in_compiler == 2:
+                op = compiler_end(op, "ccompiler")
+            self.in_compiler = 0
+            return op
+        return super().visit_call(call)
+
+
+class WholeGraphAnnotator(ExprMutator):
+    """
+    An annotator that creates a compiler for an entire graph.
+    """
+
+    def __init__(self, compiler):
+        super(WholeGraphAnnotator, self).__init__()
+        self.compiler = compiler
+        self.last_call = True
+
+    def visit_call(self, call):
+        curr_last = self.last_call
+        self.last_call = False
+
+        params = []
+        for arg in call.args:
+            param = super().visit(arg)
+            if isinstance(param, relay.expr.Var):
+                param = compiler_begin(param, self.compiler)
+            params.append(param)
+
+        new_call = relay.Call(call.op, params, call.attrs)
+        if curr_last:
+            new_call = compiler_end(new_call, self.compiler)
+        return new_call
+
+
+class MobileNetAnnotator(ExprMutator):
 
 Review comment:
   Is a class like this an alternative for adding annotate_compiler.py under relay/op/contrib/ ? Or annotate_compiler.py is always required?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361613672
 
 

 ##########
 File path: src/relay/pass/annotate_compiler.cc
 ##########
 @@ -0,0 +1,102 @@
+/*
+ * 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/pass/annotate_compiler.cc
+ * \brief Wraps a call with compiler_begin and compiler_end to indicate that
+ * the op of this call node will use external compiler.
+ */
+
+#include <tvm/operation.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/op_attr_types.h>
+#include <tvm/relay/transform.h>
+
+namespace tvm {
+namespace relay {
+namespace annotate_compiler {
+
+// A helper class to insert annotation boundaries for a program region that will
+// be handled by a specific compiler.
+class AnnotateCompilerWrapper : public ExprMutator {
+ public:
+  explicit AnnotateCompilerWrapper(const std::string& compiler) : compiler_(compiler) {}
+
+  Expr VisitExpr_(const CallNode* cn) {
+    auto new_e = ExprMutator::VisitExpr_(cn);
+
+    Call call = Downcast<Call>(new_e);
+    static auto fannotate = Op::GetAttr<FTVMAnnotateCompiler>("FTVMAnnotateCompiler");
+    Op op = Downcast<Op>(call->op);
+    CHECK(op.operator->());
 
 Review comment:
   I think you can remove .operator->()

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
comaniac commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569195854
 
 
   > @zhiics When I run the mobilenet test and dump the annotated graph, batch norm is not converted to dnnl one.
   > 
   > ```
   >   %150 = fn (%dnnl_input75: Tensor[(1, 1024, 7, 7), float32], %dnnl_input76: Tensor[(1024, 1, 3, 3), float32], Compiler="dnnl", ExternalSymbol="dnnl_4", Primitive=1) -> Tensor[(1, 1024, 7, 7), float32] {
   >     nn.conv2d(%dnnl_input75, %dnnl_input76, padding=[1, 1], groups=1024, channels=1024, kernel_size=[3, 3]) /* ty=Tensor[(1, 1024, 7, 7), float32] */
   >   };
   >   %151 = %150(%149, %separable_conv_block_13_depthwise_conv1_weight) /* ty=Tensor[(1, 1024, 7, 7), float32] */;
   >   %152 = nn.batch_norm(%151, %separable_conv_block_13_bn1_gamma, %separable_conv_block_13_bn1_beta, %separable_conv_block_13_bn1_moving_mean, %separable_conv_block_13_bn1_moving_var) /* ty=(Tensor[(1, 1024, 7, 7), float32], Tensor[(1024), float32], Tensor[(1024), float32]) */;
   >   %153 = %152.0;
   >   %154 = fn (%dnnl_input77: Tensor[(1, 1024, 7, 7), float32], Compiler="dnnl", ExternalSymbol="dnnl_3", Primitive=1) -> Tensor[(1, 1024, 7, 7), float32] {
   >     nn.relu(%dnnl_input77) /* ty=Tensor[(1, 1024, 7, 7), float32] */
   >   };
   > ```
   > 
   The BN op should be turned off in DNNL codegen because we haven't supported multiple outputs yet.
   
   > It is probably because batch norm is decomposed during SimplifyInference. Is batch norm supposed to be around during external codegen time? I think it should be. I don't hit "else if (IsOp(call, "nn.batch_norm")" condition [here](https://github.com/apache/incubator-tvm/blob/master/src/relay/backend/contrib/dnnl/codegen.cc#L73) during mobilenet test.
   > 
   The BN op will not be decomposed if it is marked as an external op.
   
   > I want to see an example of matching a pattern like Conv + BN + Relu and translate them to dnnl's fused op. It would be great if you could do that in this or future PR, otherwise I'll do it as a practice and send a PR :)
   Op fusion is not considered in this PR. It should be follow-up work after this PR is merged.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361618437
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
 
 Review comment:
   A minor issue, but if node can belong to multiple this->subgraphs_, this could lead to non determinism, since subgraphs are ordered by pointers. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361616089
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
+      }
+    }
+    return nullptr;
+  }
+
+  void MergeSubgraph(Subgraph* subgraph1, Subgraph* subgraph2) {
+    if (subgraph1 == subgraph2) {
+      return;
+    }
+
+    // Merge subgraph 2 to subgraph 1 and erase subgraph 2.
+    subgraph1->nodes.insert(subgraph2->nodes.begin(), subgraph2->nodes.end());
+    for (auto arg : subgraph2->args) {
+      subgraph1->args.push_back(arg);
+    }
+    this->subgraphs_.erase(subgraph2);
+  }
+
+  void AddToSubgraph(Subgraph* subgraph, const Expr expr) {
+    auto subgraph2 = GetSubgraph(expr);
+    if (subgraph2) {
+      MergeSubgraph(subgraph, subgraph2);
+    } else {
+      subgraph->nodes.insert(expr);
+    }
+  }
+
+  Expr VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      // Propogate subgraph to arguments
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (subgraph) {
+        for (auto arg : call->args) {
+          AddToSubgraph(subgraph, arg);
+        }
+      }
+      return ExprMutator::VisitExpr_(call);
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      // The annotation node is inserted on edge so it must have only one argument.
+      CHECK_EQ(call->args.size(), 1U);
+
+      // Traverse the rest graph.
+      auto input_expr = VisitExpr(call->args[0]);
+
+      // Replace the begin annotation with an external call input variable.
+      auto compiler_attrs = call->attrs.as<CompilerAttrs>();
+      auto var = VarNode::make(compiler_attrs->compiler + "_input" + std::to_string(var_id_++),
+                               input_expr->checked_type_);
+
+      // Find the corresponding subgraph and add the argument.
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (!subgraph) {
+        throw Error(RELAY_ERROR("Cannot find the corresponding subgraph for start annotation:\n"
+                                << AsText(GetRef<Call>(call), false)));
+      }
+      subgraph->args.push_back({var, input_expr});
+      return std::move(var);
+    } else {
+      CHECK(GetRef<Op>(op_node) == Op::Get("annotation.compiler_end"));
+      // The annotation node is inserted on edge so it must have only one argument.
+      CHECK_EQ(call->args.size(), 1U);
+
+      auto compiler_attrs = call->attrs.as<CompilerAttrs>();
+
+      // Check if the argument is already belonged to an exist subgraph
+      auto subgraph = GetSubgraph(call->args[0]);
+      if (!subgraph) {
+        auto ret = this->subgraphs_.emplace(new Subgraph());
+        subgraph = *ret.first;
+        subgraph->nodes.insert(call->args[0]);
+        subgraph->id = this->subgraph_id_++;
+      }
+      subgraph->nodes.insert(GetRef<Call>(call));
+
+      // Traverse towarding to subgraph inputs.
 
 Review comment:
   towarding to -> toward

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361611329
 
 

 ##########
 File path: python/tvm/relay/op/contrib/ccompiler/__init__.py
 ##########
 @@ -0,0 +1,20 @@
+# 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.
+# pylint: disable=wildcard-import
+"""Neural network related operators."""
 
 Review comment:
   remove this doc string

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361727260
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
 
 Review comment:
   Please see #4594, I sent a PR to refactor all places.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361619198
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
+      }
+    }
+    return nullptr;
+  }
+
+  void MergeSubgraph(Subgraph* subgraph1, Subgraph* subgraph2) {
+    if (subgraph1 == subgraph2) {
+      return;
+    }
+
+    // Merge subgraph 2 to subgraph 1 and erase subgraph 2.
+    subgraph1->nodes.insert(subgraph2->nodes.begin(), subgraph2->nodes.end());
+    for (auto arg : subgraph2->args) {
+      subgraph1->args.push_back(arg);
+    }
+    this->subgraphs_.erase(subgraph2);
+  }
+
+  void AddToSubgraph(Subgraph* subgraph, const Expr expr) {
+    auto subgraph2 = GetSubgraph(expr);
+    if (subgraph2) {
+      MergeSubgraph(subgraph, subgraph2);
+    } else {
+      subgraph->nodes.insert(expr);
+    }
+  }
+
+  Expr VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      // Propogate subgraph to arguments
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (subgraph) {
+        for (auto arg : call->args) {
+          AddToSubgraph(subgraph, arg);
+        }
+      }
+      return ExprMutator::VisitExpr_(call);
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      // The annotation node is inserted on edge so it must have only one argument.
+      CHECK_EQ(call->args.size(), 1U);
+
+      // Traverse the rest graph.
+      auto input_expr = VisitExpr(call->args[0]);
+
+      // Replace the begin annotation with an external call input variable.
+      auto compiler_attrs = call->attrs.as<CompilerAttrs>();
+      auto var = VarNode::make(compiler_attrs->compiler + "_input" + std::to_string(var_id_++),
+                               input_expr->checked_type_);
+
+      // Find the corresponding subgraph and add the argument.
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (!subgraph) {
+        throw Error(RELAY_ERROR("Cannot find the corresponding subgraph for start annotation:\n"
+                                << AsText(GetRef<Call>(call), false)));
+      }
+      subgraph->args.push_back({var, input_expr});
+      return std::move(var);
+    } else {
+      CHECK(GetRef<Op>(op_node) == Op::Get("annotation.compiler_end"));
 
 Review comment:
   better to use IsOp()

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361727512
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
 
 Review comment:
   I don't think this is allowed, as we cannot really executed one op on multiple devices. @comaniac 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361728317
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
 
 Review comment:
   No it's not allowed. The semantic will definitely incorrect if an op belongs to multiple subgraphs.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361605864
 
 

 ##########
 File path: include/tvm/relay/attrs/annotation.h
 ##########
 @@ -57,6 +57,19 @@ struct CastHintAttrs : public tvm::AttrsNode<CastHintAttrs> {
   }
 };
 
+/*!
+ * \brief Options for the operators used to annotate a compiler.
+ */
+struct CompilerAttrs : public tvm::AttrsNode<CompilerAttrs> {
+  /*! \brief The 3rd party compiler for code generation. */
+  std::string compiler;
+
+  TVM_DECLARE_ATTRS(CompilerAttrs, "relay.attrs.CompilerAttrs") {
+    TVM_ATTR_FIELD(compiler)
+      .describe("The 3rd compiler used for code generation.");
 
 Review comment:
   third party

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361616973
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
+      }
+    }
+    return nullptr;
+  }
+
+  void MergeSubgraph(Subgraph* subgraph1, Subgraph* subgraph2) {
+    if (subgraph1 == subgraph2) {
+      return;
+    }
+
+    // Merge subgraph 2 to subgraph 1 and erase subgraph 2.
+    subgraph1->nodes.insert(subgraph2->nodes.begin(), subgraph2->nodes.end());
+    for (auto arg : subgraph2->args) {
+      subgraph1->args.push_back(arg);
 
 Review comment:
   maybe better to use std::vector::insert

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361605888
 
 

 ##########
 File path: tests/python/relay/test_pass_partition_graph.py
 ##########
 @@ -0,0 +1,391 @@
+# 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.
+"""Unit tests for graph partitioning."""
+import os
+import sys
+import numpy as np
+import pytest
+
+import tvm
+import tvm.relay.testing
+import tvm.relay.transform
+from tvm import relay
+from tvm.contrib import util
+from tvm.relay.annotation import compiler_begin, compiler_end
+from tvm.relay.expr_functor import ExprMutator
+
+
+class CcompilerAnnotator(ExprMutator):
+    """
+    A simple annotator that creates the following program:
+           |
+      -- begin --
+           |
+          add
+           |
+        subtract
+           |
+        multiply
+           |
+       -- end --
+           |
+    """
+
+    def __init__(self):
+        super(CcompilerAnnotator, self).__init__()
+        self.in_compiler = 0
+
+    def visit_call(self, call):
+        if call.op.name == "add":  # Annotate begin at args
+            if self.in_compiler == 1:
+                lhs = compiler_begin(super().visit(call.args[0]), "ccompiler")
+                rhs = compiler_begin(super().visit(call.args[1]), "ccompiler")
+                op = relay.add(lhs, rhs)
+                self.in_compiler = 2
+                return op
+        elif call.op.name == "subtract":
+            if self.in_compiler == 1:
+                lhs = super().visit(call.args[0])
+                rhs = super().visit(call.args[1])
+                if isinstance(lhs, relay.expr.Var):
+                    lhs = compiler_begin(lhs, "ccompiler")
+                if isinstance(rhs, relay.expr.Var):
+                    rhs = compiler_begin(rhs, "ccompiler")
+                return relay.subtract(lhs, rhs)
+        elif call.op.name == "multiply":  # Annotate end at output
+            self.in_compiler = 1
+            lhs = super().visit(call.args[0])
+            rhs = super().visit(call.args[1])
+            if isinstance(lhs, relay.expr.Var):
+                lhs = compiler_begin(lhs, "ccompiler")
+            if isinstance(rhs, relay.expr.Var):
+                rhs = compiler_begin(rhs, "ccompiler")
+            op = relay.multiply(lhs, rhs)
+            if self.in_compiler == 2:
+                op = compiler_end(op, "ccompiler")
+            self.in_compiler = 0
+            return op
+        return super().visit_call(call)
+
+
+class WholeGraphAnnotator(ExprMutator):
+    """
+    An annotator that creates a compiler for an entire graph.
+    """
+
+    def __init__(self, compiler):
+        super(WholeGraphAnnotator, self).__init__()
+        self.compiler = compiler
+        self.last_call = True
+
+    def visit_call(self, call):
+        curr_last = self.last_call
+        self.last_call = False
+
+        params = []
+        for arg in call.args:
+            param = super().visit(arg)
+            if isinstance(param, relay.expr.Var):
+                param = compiler_begin(param, self.compiler)
+            params.append(param)
+
+        new_call = relay.Call(call.op, params, call.attrs)
+        if curr_last:
+            new_call = compiler_end(new_call, self.compiler)
+        return new_call
+
+
+class MobileNetAnnotator(ExprMutator):
 
 Review comment:
   Yes. This could be an alternative solution if you want to have subgraphs with more than one ops. As you can see in this unit test, the test `test_extern_ccompiler_single_op`  directly calls `relay.build_extern_compiler` without doing anything else. In this case, the annotate_compiler will be invoked to partition the graph and generate one-op subgraphs. On the other hand, the test like `test_multi_node_compiler` and `test_extern_dnnl` manually apply customized annotators, so they have to explicitly call `PartitionGraph()` as you said to partition and generate multi-op subgraphs.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361619589
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
+      }
+    }
+    return nullptr;
+  }
+
+  void MergeSubgraph(Subgraph* subgraph1, Subgraph* subgraph2) {
+    if (subgraph1 == subgraph2) {
+      return;
+    }
+
+    // Merge subgraph 2 to subgraph 1 and erase subgraph 2.
+    subgraph1->nodes.insert(subgraph2->nodes.begin(), subgraph2->nodes.end());
+    for (auto arg : subgraph2->args) {
+      subgraph1->args.push_back(arg);
+    }
+    this->subgraphs_.erase(subgraph2);
+  }
+
+  void AddToSubgraph(Subgraph* subgraph, const Expr expr) {
+    auto subgraph2 = GetSubgraph(expr);
+    if (subgraph2) {
+      MergeSubgraph(subgraph, subgraph2);
+    } else {
+      subgraph->nodes.insert(expr);
+    }
+  }
+
+  Expr VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      // Propogate subgraph to arguments
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (subgraph) {
+        for (auto arg : call->args) {
+          AddToSubgraph(subgraph, arg);
+        }
+      }
+      return ExprMutator::VisitExpr_(call);
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      // The annotation node is inserted on edge so it must have only one argument.
+      CHECK_EQ(call->args.size(), 1U);
+
+      // Traverse the rest graph.
+      auto input_expr = VisitExpr(call->args[0]);
+
+      // Replace the begin annotation with an external call input variable.
+      auto compiler_attrs = call->attrs.as<CompilerAttrs>();
+      auto var = VarNode::make(compiler_attrs->compiler + "_input" + std::to_string(var_id_++),
+                               input_expr->checked_type_);
+
+      // Find the corresponding subgraph and add the argument.
+      auto subgraph = GetSubgraph(GetRef<Call>(call));
+      if (!subgraph) {
+        throw Error(RELAY_ERROR("Cannot find the corresponding subgraph for start annotation:\n"
+                                << AsText(GetRef<Call>(call), false)));
+      }
+      subgraph->args.push_back({var, input_expr});
+      return std::move(var);
+    } else {
+      CHECK(GetRef<Op>(op_node) == Op::Get("annotation.compiler_end"));
+      // The annotation node is inserted on edge so it must have only one argument.
+      CHECK_EQ(call->args.size(), 1U);
+
+      auto compiler_attrs = call->attrs.as<CompilerAttrs>();
+
+      // Check if the argument is already belonged to an exist subgraph
+      auto subgraph = GetSubgraph(call->args[0]);
+      if (!subgraph) {
+        auto ret = this->subgraphs_.emplace(new Subgraph());
 
 Review comment:
   better to use shard_ptr

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569217109
 
 
   For others who might be interested in this PR, here is a before/after of partitioning in test_multi_node_compiler().
   
   Before
   ```
   fn (%x: Tensor[(10, 10), float32], %w0: Tensor[(10, 10), float32], %w1: Tensor[(10, 10), float32], %w2: Tensor[(10, 10), float32], %w3: Tensor[(10, 10), float32], %w4: Tensor[(10, 10), float32], %w5: Tensor[(10, 10), float32], %w6: Tensor[(10, 10), float32], %w7: Tensor[(10, 10), float32]) -> Tensor[(30, 10), float32] {
     %0 = annotation.compiler_begin(%x, meta[relay.attrs.CompilerAttrs][0]) /* ty=Tensor[(10, 10), float32] */;
     %1 = annotation.compiler_begin(%w0, meta[relay.attrs.CompilerAttrs][1]) /* ty=Tensor[(10, 10), float32] */;
     %2 = add(%0, %1) /* ty=Tensor[(10, 10), float32] */;
     %3 = annotation.compiler_begin(%w1, meta[relay.attrs.CompilerAttrs][2]) /* ty=Tensor[(10, 10), float32] */;
     %4 = subtract(%2, %3) /* ty=Tensor[(10, 10), float32] */;
     %5 = annotation.compiler_begin(%w2, meta[relay.attrs.CompilerAttrs][3]) /* ty=Tensor[(10, 10), float32] */;
     %6 = multiply(%4, %5) /* ty=Tensor[(10, 10), float32] */;
     %7 = annotation.compiler_end(%6, meta[relay.attrs.CompilerAttrs][4]) /* ty=Tensor[(10, 10), float32] */;
     %8 = annotation.compiler_begin(%x, meta[relay.attrs.CompilerAttrs][5]) /* ty=Tensor[(10, 10), float32] */;
     %9 = annotation.compiler_begin(%w3, meta[relay.attrs.CompilerAttrs][6]) /* ty=Tensor[(10, 10), float32] */;
     %10 = add(%8, %9) /* ty=Tensor[(10, 10), float32] */;
     %11 = annotation.compiler_begin(%w4, meta[relay.attrs.CompilerAttrs][7]) /* ty=Tensor[(10, 10), float32] */;
     %12 = subtract(%10, %11) /* ty=Tensor[(10, 10), float32] */;
     %13 = annotation.compiler_begin(%w5, meta[relay.attrs.CompilerAttrs][8]) /* ty=Tensor[(10, 10), float32] */;
     %14 = multiply(%12, %13) /* ty=Tensor[(10, 10), float32] */;
     %15 = annotation.compiler_end(%14, meta[relay.attrs.CompilerAttrs][9]) /* ty=Tensor[(10, 10), float32] */;
     %16 = add(%x, %w6) /* ty=Tensor[(10, 10), float32] */;
     %17 = subtract(%16, %w7) /* ty=Tensor[(10, 10), float32] */;
     %18 = (%7, %15, %17);
     concatenate(%18) /* ty=Tensor[(30, 10), float32] */
   }
   ```
   
   After
   ```
   def @main(%x: Tensor[(10, 10), float32], %w0: Tensor[(10, 10), float32], %w1: Tensor[(10, 10), float32], %w2: Tensor[(10, 10), float32], %w3: Tensor[(10, 10), float32], %w4: Tensor[(10, 10), float32], %w5: Tensor[(10, 10), float32], %w6: Tensor[(10, 10), float32], %w7: Tensor[(10, 10), float32]) -> Tensor[(30, 10), float32] {
     %2 = fn (%ccompiler_input0: Tensor[(10, 10), float32], %ccompiler_input1: Tensor[(10, 10), float32], %ccompiler_input2: Tensor[(10, 10), float32], %ccompiler_input3: Tensor[(10, 10), float32], Compiler="ccompiler", ExternalSymbol="ccompiler_0", Primitive=1) -> Tensor[(10, 10), float32] {
       %0 = add(%ccompiler_input0, %ccompiler_input1) /* ty=Tensor[(10, 10), float32] */;
       %1 = subtract(%0, %ccompiler_input2) /* ty=Tensor[(10, 10), float32] */;
       multiply(%1, %ccompiler_input3) /* ty=Tensor[(10, 10), float32] */
     };
     %3 = %2(%x, %w0, %w1, %w2) /* ty=Tensor[(10, 10), float32] */;
     %6 = fn (%ccompiler_input4: Tensor[(10, 10), float32], %ccompiler_input5: Tensor[(10, 10), float32], %ccompiler_input6: Tensor[(10, 10), float32], %ccompiler_input7: Tensor[(10, 10), float32], Compiler="ccompiler", ExternalSymbol="ccompiler_1", Primitive=1) -> Tensor[(10, 10), float32] {
       %4 = add(%ccompiler_input4, %ccompiler_input5) /* ty=Tensor[(10, 10), float32] */;
       %5 = subtract(%4, %ccompiler_input6) /* ty=Tensor[(10, 10), float32] */;
       multiply(%5, %ccompiler_input7) /* ty=Tensor[(10, 10), float32] */
     };
     %7 = %6(%x, %w3, %w4, %w5) /* ty=Tensor[(10, 10), float32] */;
     %8 = add(%x, %w6) /* ty=Tensor[(10, 10), float32] */;
     %9 = subtract(%8, %w7) /* ty=Tensor[(10, 10), float32] */;
     %10 = (%3, %7, %9);
     concatenate(%10) /* ty=Tensor[(30, 10), float32] */
   }
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
comaniac commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569204482
 
 
   > Ideally whether to keep batch norm or not should be configurable. Actually for my use case, I want batch norm to be decomposed and its multiplication folded into previous conv, so that I don't have to worry about supporting batch norm on my end. But I expect others have a special op for handling fused Conv + BN + Relu.
   > 
   > UPDATE: I realized this is already achieved by allowing each backend to have its own annotate_compiler registration.
   
   Yes you are right. This PR provides an interface (annotation) for specialized codegen to do subgraph pattern matching for fusion or similar optimization. However, we also realize that this may be painful for developers and it might be better to provide a more convenient pattern matching for widely used cases (e.g., Conv+BN+ReLU as you mentioned). Our follow-up plans to focus more on the graph partitioning algorithm, and this is one of the cases we want to cover.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361611329
 
 

 ##########
 File path: python/tvm/relay/op/contrib/ccompiler/__init__.py
 ##########
 @@ -0,0 +1,20 @@
+# 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.
+# pylint: disable=wildcard-import
+"""Neural network related operators."""
 
 Review comment:
   remove this document

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-568823854
 
 
   @zhiics when I run test_pass_partition_graph.py, I get a warning which says
   
   ```
   add in ccompiler is not registered. Fallback to CPU
   multiply in ccompiler is not registered. Fallback to CPU
   add in ccompiler is not registered. Fallback to CPU
   subtract in ccompiler is not registered. Fallback to CPU
   ```
   
   Is this expected? Or do I need to configure something on cmake?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569199138
 
 
   @comaniac ok thanks, I'll study the code.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-570856097
 
 
   It would be great to think a bit more about the API. Personally, I don't like the fact that we are adding new build functions (build_extern_compiler). We should advocate the pipeline API more and avoid the general interface.
   
   Given that this is not our final form, I would suggest we only keep the pipeline API. e.g.
   
   ```python
   mod = relay.transform.AnnotateExternCompiler("dnnl")(mod)
   ```
   
   Similarly, the term register_annotate_compiler seems to be quite arbitrary. Is it mainly built for dnnl? should it have a dnnl namespace instead, should we just make use of set_attr for now?
   
   
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361727512
 
 

 ##########
 File path: src/relay/pass/partition_graph.cc
 ##########
 @@ -0,0 +1,376 @@
+/*
+ * 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/pass/partition.cc
+ *
+ * \brief Partition an input function into multiple functions according based
+ * on the inserted annotation nodes (i.e. compiler_begin and compiler_end).
+ * These nodes are used as boundaries to partition the Relay function into
+ * multiple regions that can be offloaded to different accelerators/backends.
+ *
+ * Each of these paritioned functions, a.k.a subgraphs, will be viewed as
+ * external functions, and they will use the provided compiler for codegen.
+ */
+
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace relay {
+namespace partitioning {
+
+/*!
+ * \brief The subgraph properties for partitioning.
+ */
+struct Subgraph {
+  /*! \brief The subgraph ID. */
+  int id;
+
+  /*! \brief The input arguments of this subgraph. */
+  std::vector<std::pair<Var, Expr>> args;
+
+  /*! \brief Nodes in this subgraph. */
+  std::unordered_set<Expr, ExprHash, ExprEqual> nodes;
+};
+
+/*!
+ * \brief The checker that verifies if a Relay program is annotated correctly
+ * for partitioning.
+ */
+class AnnotationChecker : public ExprVisitor {
+ public:
+  bool Check() {
+    if (!this->found_start && !this->found_end) {
+      LOG(WARNING) << "No compiler annotation found";
+    } else if (!this->found_start) {
+      LOG(ERROR) << "compiler_begin annotation is missing";
+      return false;
+    } else if (!this->found_end) {
+      LOG(ERROR) << "compiler_end annotation is missing";
+      return false;
+    }
+    return true;
+  }
+
+  void VisitExpr_(const CallNode* call) final {
+    auto op_node = call->op.as<OpNode>();
+    if (op_node == nullptr || call->attrs.as<CompilerAttrs>() == nullptr) {
+      return;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_begin")) {
+      this->found_start = true;
+    } else if (GetRef<Op>(op_node) == Op::Get("annotation.compiler_end")) {
+      this->found_end = true;
+    }
+  }
+
+ private:
+  bool found_start = false;
+  bool found_end = false;
+};
+
+/*! \brief This class partitions the expr labeled with begin and end annoations
+ * into function containing multiple regions. Each region is labeled with
+ * a compiler attribute so that it will be handled by any compilers that are not
+ * in the TVM stack.
+ *
+ * TODO(@zhiics) This following algorithm is not adequate to handle all cases,
+ * i.e. multiple `compiler_end` nodes.
+ */
+class Partitioner : public ExprMutator {
+ public:
+  Subgraph* GetSubgraph(const Expr node) {
+    for (auto candidate : this->subgraphs_) {
+      if (candidate->nodes.find(node) != candidate->nodes.end()) {
+        return candidate;
 
 Review comment:
   I don't think this is allowed, as we cannot really execute one op on multiple devices. @comaniac 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac edited a comment on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
comaniac edited a comment on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569198065
 
 
   > If BN is not decomposed, why am I not seeing dnnl_bn generated?
   
   It is not marked as external because `batch_norm` is not defined in python/tvm/relay/op/contrib/ccompiler/annotate_compiler.py
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi edited a comment on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi edited a comment on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569201036
 
 
   Ideally whether to keep batch norm or not should be configurable. Actually for my use case, I want batch norm to be decomposed and its multiplication folded into previous conv, so that I don't have to worry about supporting batch norm on my end. But I expect others have a special op for handling fused Conv + BN + Relu.
   
   UPDATE: I realized this is already achieved by allowing each backend to have its own annotate_compiler registration.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361605888
 
 

 ##########
 File path: tests/python/relay/test_pass_partition_graph.py
 ##########
 @@ -0,0 +1,391 @@
+# 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.
+"""Unit tests for graph partitioning."""
+import os
+import sys
+import numpy as np
+import pytest
+
+import tvm
+import tvm.relay.testing
+import tvm.relay.transform
+from tvm import relay
+from tvm.contrib import util
+from tvm.relay.annotation import compiler_begin, compiler_end
+from tvm.relay.expr_functor import ExprMutator
+
+
+class CcompilerAnnotator(ExprMutator):
+    """
+    A simple annotator that creates the following program:
+           |
+      -- begin --
+           |
+          add
+           |
+        subtract
+           |
+        multiply
+           |
+       -- end --
+           |
+    """
+
+    def __init__(self):
+        super(CcompilerAnnotator, self).__init__()
+        self.in_compiler = 0
+
+    def visit_call(self, call):
+        if call.op.name == "add":  # Annotate begin at args
+            if self.in_compiler == 1:
+                lhs = compiler_begin(super().visit(call.args[0]), "ccompiler")
+                rhs = compiler_begin(super().visit(call.args[1]), "ccompiler")
+                op = relay.add(lhs, rhs)
+                self.in_compiler = 2
+                return op
+        elif call.op.name == "subtract":
+            if self.in_compiler == 1:
+                lhs = super().visit(call.args[0])
+                rhs = super().visit(call.args[1])
+                if isinstance(lhs, relay.expr.Var):
+                    lhs = compiler_begin(lhs, "ccompiler")
+                if isinstance(rhs, relay.expr.Var):
+                    rhs = compiler_begin(rhs, "ccompiler")
+                return relay.subtract(lhs, rhs)
+        elif call.op.name == "multiply":  # Annotate end at output
+            self.in_compiler = 1
+            lhs = super().visit(call.args[0])
+            rhs = super().visit(call.args[1])
+            if isinstance(lhs, relay.expr.Var):
+                lhs = compiler_begin(lhs, "ccompiler")
+            if isinstance(rhs, relay.expr.Var):
+                rhs = compiler_begin(rhs, "ccompiler")
+            op = relay.multiply(lhs, rhs)
+            if self.in_compiler == 2:
+                op = compiler_end(op, "ccompiler")
+            self.in_compiler = 0
+            return op
+        return super().visit_call(call)
+
+
+class WholeGraphAnnotator(ExprMutator):
+    """
+    An annotator that creates a compiler for an entire graph.
+    """
+
+    def __init__(self, compiler):
+        super(WholeGraphAnnotator, self).__init__()
+        self.compiler = compiler
+        self.last_call = True
+
+    def visit_call(self, call):
+        curr_last = self.last_call
+        self.last_call = False
+
+        params = []
+        for arg in call.args:
+            param = super().visit(arg)
+            if isinstance(param, relay.expr.Var):
+                param = compiler_begin(param, self.compiler)
+            params.append(param)
+
+        new_call = relay.Call(call.op, params, call.attrs)
+        if curr_last:
+            new_call = compiler_end(new_call, self.compiler)
+        return new_call
+
+
+class MobileNetAnnotator(ExprMutator):
 
 Review comment:
   Yes. This could be an alternative solution if you want to have subgraphs with more than one ops. As you can see in this unit test, the test `test_extern_ccompiler_single_op`  directly calls `relay.build_extern_compiler` without doing anything else. In this case, the annotate_compiler will be invoked to partition the graph and generate one-op subgraphs. On the other hand, tests like `test_multi_node_compiler` and `test_extern_dnnl` manually apply customized annotators, so they have to explicitly call `PartitionGraph()` as you said to partition and generate multi-op subgraphs.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361613372
 
 

 ##########
 File path: python/tvm/relay/op/contrib/dnnl/__init__.py
 ##########
 @@ -0,0 +1,20 @@
+# 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.
+# pylint: disable=wildcard-import
+"""Neural network related operators."""
 
 Review comment:
   remove the doc string

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361611845
 
 

 ##########
 File path: python/tvm/relay/op/contrib/ccompiler/annotate_compiler.py
 ##########
 @@ -0,0 +1,39 @@
+# 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.
+# pylint: disable=invalid-name, unused-argument
+"""C/C++ compiler supported operators."""
+from __future__ import absolute_import
+
+def conv2d(attrs, args):
+    """Check if the external C source codegen should be used.
+    """
+    return False
 
 Review comment:
   if False, we don't need to explicitly specify this right? I think it is fine if this is pedagogical purpose

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361613581
 
 

 ##########
 File path: src/relay/pass/annotate_compiler.cc
 ##########
 @@ -0,0 +1,102 @@
+/*
+ * 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/pass/annotate_compiler.cc
+ * \brief Wraps a call with compiler_begin and compiler_end to indicate that
+ * the op of this call node will use external compiler.
+ */
+
+#include <tvm/operation.h>
+#include <tvm/relay/attrs/annotation.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/op_attr_types.h>
+#include <tvm/relay/transform.h>
+
+namespace tvm {
+namespace relay {
+namespace annotate_compiler {
+
+// A helper class to insert annotation boundaries for a program region that will
+// be handled by a specific compiler.
+class AnnotateCompilerWrapper : public ExprMutator {
+ public:
+  explicit AnnotateCompilerWrapper(const std::string& compiler) : compiler_(compiler) {}
+
+  Expr VisitExpr_(const CallNode* cn) {
+    auto new_e = ExprMutator::VisitExpr_(cn);
+
+    Call call = Downcast<Call>(new_e);
+    static auto fannotate = Op::GetAttr<FTVMAnnotateCompiler>("FTVMAnnotateCompiler");
+    Op op = Downcast<Op>(call->op);
+    CHECK(op.operator->());
+
+    if (fannotate.count(op)) {
+      bool external = fannotate[op](call->attrs, call->args, compiler_);
+      if (external) {
+        tvm::Array<tvm::relay::Expr> compiler_begins;
+        for (const auto& it : call->args) {
+          const auto* begin_op =
+            runtime::Registry::Get("relay.op.annotation._make.compiler_begin");
+          CHECK(begin_op);
+          Expr begin = (*begin_op)(it, compiler_);
+          compiler_begins.push_back(begin);
+        }
+        Expr update_call = CallNode::make(call->op, compiler_begins, call->attrs);
+        const auto* end_op =
+          runtime::Registry::Get("relay.op.annotation._make.compiler_end");
+        CHECK(end_op);
+        Expr end = (*end_op)(update_call, compiler_);
+        return end;
+      }
+    } else {
+      LOG(WARNING) << op.operator->()->name << " in " << compiler_ << " is not registered";
 
 Review comment:
   op->name

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics merged pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics merged pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] MarisaKirisame commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r362293610
 
 

 ##########
 File path: include/tvm/relay/attrs/annotation.h
 ##########
 @@ -57,6 +57,19 @@ struct CastHintAttrs : public tvm::AttrsNode<CastHintAttrs> {
   }
 };
 
+/*!
+ * \brief Options for the operators used to annotate a compiler.
+ */
+struct CompilerAttrs : public tvm::AttrsNode<CompilerAttrs> {
+  /*! \brief A 3rd party compiler for code generation. */
+  std::string compiler;
+
+  TVM_DECLARE_ATTRS(CompilerAttrs, "relay.attrs.CompilerAttrs") {
+    TVM_ATTR_FIELD(compiler)
+      .describe("A 3rd compiler used for code generation.");
 
 Review comment:
   ```suggestion
         .describe("A 3rd party compiler used for code generation.");
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics edited a comment on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics edited a comment on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-573520559
 
 
   As discussed in https://discuss.tvm.ai/t/rfc-naming-and-api-signature-for-external-compilation/5292, let's remove the annotation template for now. Instead, we only focus on partitioning and expect developers to annotate the program first.
   
   Some example annotators that leverage the pass manager are added to help developers write their own annotator.
   
   Followup PRs will be sent to help annotation which may need more discussion and thoughts on how to combine together with OpStrategy.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-568921493
 
 
   @masahi 
   
   > Obviously operator fusion should be disabled for the subgraph that would be sent to external tools, but for other nodes that run on CPU I want operator fusion to work as usual. Is that possible?
   
   Yes, this is possible. I just added a unit test for this. Please check it out.
   
   > I want subgraphs I get from this pass to be already quantized, so the quantization pass should happen before this pass. Are quantization and this pass going to work without any issues?
   
   Yes, you should be able customize your opt pass pipeline. For example:
   ```python
   # Given a relay module, mod
   with relay.quantize.qconfig(...):
       q_mod = relay.quantize.quantize(mod, params)
   
   seq1 = [relay.transform.xx, relay.transform.yy]
   with relay.build_config(...)
       mod = seq1(q_mod)
   
   mod = relay.build_extern_compiler(mod, "ccompiler")
   
   relaly.build_config(...):
       graph, lib, params = relay.build(mod, "llvm")
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-573980033
 
 
   @tqchen We removed the annotation part, any other outstanding concerns?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-568946092
 
 
   @zhiics great, 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#discussion_r361598019
 
 

 ##########
 File path: tests/python/relay/test_pass_partition_graph.py
 ##########
 @@ -0,0 +1,391 @@
+# 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.
+"""Unit tests for graph partitioning."""
+import os
+import sys
+import numpy as np
+import pytest
+
+import tvm
+import tvm.relay.testing
+import tvm.relay.transform
+from tvm import relay
+from tvm.contrib import util
+from tvm.relay.annotation import compiler_begin, compiler_end
+from tvm.relay.expr_functor import ExprMutator
+
+
+class CcompilerAnnotator(ExprMutator):
+    """
+    A simple annotator that creates the following program:
+           |
+      -- begin --
+           |
+          add
+           |
+        subtract
+           |
+        multiply
+           |
+       -- end --
+           |
+    """
+
+    def __init__(self):
+        super(CcompilerAnnotator, self).__init__()
+        self.in_compiler = 0
+
+    def visit_call(self, call):
+        if call.op.name == "add":  # Annotate begin at args
+            if self.in_compiler == 1:
+                lhs = compiler_begin(super().visit(call.args[0]), "ccompiler")
+                rhs = compiler_begin(super().visit(call.args[1]), "ccompiler")
+                op = relay.add(lhs, rhs)
+                self.in_compiler = 2
+                return op
+        elif call.op.name == "subtract":
+            if self.in_compiler == 1:
+                lhs = super().visit(call.args[0])
+                rhs = super().visit(call.args[1])
+                if isinstance(lhs, relay.expr.Var):
+                    lhs = compiler_begin(lhs, "ccompiler")
+                if isinstance(rhs, relay.expr.Var):
+                    rhs = compiler_begin(rhs, "ccompiler")
+                return relay.subtract(lhs, rhs)
+        elif call.op.name == "multiply":  # Annotate end at output
+            self.in_compiler = 1
+            lhs = super().visit(call.args[0])
+            rhs = super().visit(call.args[1])
+            if isinstance(lhs, relay.expr.Var):
+                lhs = compiler_begin(lhs, "ccompiler")
+            if isinstance(rhs, relay.expr.Var):
+                rhs = compiler_begin(rhs, "ccompiler")
+            op = relay.multiply(lhs, rhs)
+            if self.in_compiler == 2:
+                op = compiler_end(op, "ccompiler")
+            self.in_compiler = 0
+            return op
+        return super().visit_call(call)
+
+
+class WholeGraphAnnotator(ExprMutator):
+    """
+    An annotator that creates a compiler for an entire graph.
+    """
+
+    def __init__(self, compiler):
+        super(WholeGraphAnnotator, self).__init__()
+        self.compiler = compiler
+        self.last_call = True
+
+    def visit_call(self, call):
+        curr_last = self.last_call
+        self.last_call = False
+
+        params = []
+        for arg in call.args:
+            param = super().visit(arg)
+            if isinstance(param, relay.expr.Var):
+                param = compiler_begin(param, self.compiler)
+            params.append(param)
+
+        new_call = relay.Call(call.op, params, call.attrs)
+        if curr_last:
+            new_call = compiler_end(new_call, self.compiler)
+        return new_call
+
+
+class MobileNetAnnotator(ExprMutator):
 
 Review comment:
   Is a class like this an alternative for adding annotate_compiler.py under relay/op/contrib/ ? Or annotate_compiler.py is always required?
   
   I guess if I use a custom annotator, I have to insert PartitionGraph() manually, since I won't be using relay.build_extern_compiler() in this case.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-570870369
 
 
   @tqchen Opened here: https://discuss.tvm.ai/t/rfc-naming-and-api-signature-for-external-compilation/5292

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-570857163
 
 
   So to summary my concerns. Extern compilation support is definitely an important feature, how to design the related API will have a quite big impact for our users.
   
   While I think the related PRs have the techincal solution to deliver "a solution" for our problem. It would be really nice if we can bring a few API design options(e.g. choices of build_extern_compiler vs AnnotateCompiler, the choice of register_annotate_compiler, their signatures) as RFC discussion, this way we can make sure we design the most easy to use APIs for our developers.
   
   How about open a new discuss thread that contains those choices and try to get people's opinion on the choices?
   
   
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-569201036
 
 
   Ideally whether to keep batch norm or not should be configurable. Actually for my use case, I want batch norm to be decomposed and its multiplication folded into previous conv, so that I don't have to worry about supporting batch norm on my end. But I expect others have a special op for handling fused Conv + BN + Relu.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-568870463
 
 
   ok, fixed now.
   
   I have two questions regarding how this PR would interact with other transformation passes:
   * Obviously operator fusion should be disabled for the subgraph that would be sent to external tools, but for other nodes that run on CPU I want operator fusion to work as usual. Is that possible? 
   * I want subgraphs I get from this pass to be already quantized, so the quantization pass should happen before this pass. Are quantization and this pass going to work without any issues? 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers

Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #4570: [relay] Relay annotation and partitioning for external compilers
URL: https://github.com/apache/incubator-tvm/pull/4570#issuecomment-574338792
 
 
   Thanks @tqchen @masahi @comaniac 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services