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 2020/11/22 08:04:45 UTC

[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #6946: [Relay] Add DefuseOps pass

FrozenGene commented on a change in pull request #6946:
URL: https://github.com/apache/incubator-tvm/pull/6946#discussion_r528296240



##########
File path: src/relay/transforms/defuse_ops.cc
##########
@@ -0,0 +1,87 @@
+/*
+ * 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/transforms/defuse_ops.cc
+ * \brief This is an inverse operation of fusion pass. It transforms a fused
+ * program returned by relay::transform::FuseOps into the program before FuseOps.
+ * (i.e., x == DefuseOps(FuseOps(x)))
+ */
+
+#include <tvm/relay/attrs/transform.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+
+#include "pattern_utils.h"
+
+namespace tvm {
+namespace relay {
+
+class DefuseOpsMutator : public ExprMutator {
+ public:
+  class FuncBodyMutator : public ExprMutator {
+   public:
+    explicit FuncBodyMutator(const Array<Expr>& args) : ExprMutator() { args_ = args; }
+
+    Expr VisitExpr_(const VarNode* n) {
+      const std::string& name = n->name_hint();
+      ICHECK_EQ(name[0], 'p');

Review comment:
       `ICHECK(!name.empty() && (name[0] == 'p'))` which could make sure we will not access out of boundary because of short circuit of size check.

##########
File path: python/tvm/relay/transform/transform.py
##########
@@ -268,6 +268,18 @@ def FuseOps(fuse_opt_level=-1):
     return _ffi_api.FuseOps(fuse_opt_level)
 
 
+def DefuseOps():
+    """The inverse operation of FuseOps. It transforms a fused program returned by FuseOps into the
+    program before FuseOps. (i.e., x == DefuseOps(FuseOps(x)))
+
+    Returns
+    -------
+    ret : tvm.transform.Pass
+        The registered pass for operator fusion.

Review comment:
       fusion -> defusion

##########
File path: src/relay/transforms/defuse_ops.cc
##########
@@ -0,0 +1,87 @@
+/*
+ * 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/transforms/defuse_ops.cc
+ * \brief This is an inverse operation of fusion pass. It transforms a fused
+ * program returned by relay::transform::FuseOps into the program before FuseOps.
+ * (i.e., x == DefuseOps(FuseOps(x)))
+ */
+
+#include <tvm/relay/attrs/transform.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+
+#include "pattern_utils.h"
+
+namespace tvm {
+namespace relay {
+
+class DefuseOpsMutator : public ExprMutator {
+ public:
+  class FuncBodyMutator : public ExprMutator {
+   public:
+    explicit FuncBodyMutator(const Array<Expr>& args) : ExprMutator() { args_ = args; }
+
+    Expr VisitExpr_(const VarNode* n) {
+      const std::string& name = n->name_hint();
+      ICHECK_EQ(name[0], 'p');
+      std::string id_str = name.substr(1);
+      int id = atoi(id_str.c_str());

Review comment:
       we could use C++11 `std::stoi`,  which is safer than C style `atoi`. 

##########
File path: src/relay/transforms/defuse_ops.cc
##########
@@ -0,0 +1,87 @@
+/*
+ * 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/transforms/defuse_ops.cc
+ * \brief This is an inverse operation of fusion pass. It transforms a fused
+ * program returned by relay::transform::FuseOps into the program before FuseOps.
+ * (i.e., x == DefuseOps(FuseOps(x)))
+ */
+
+#include <tvm/relay/attrs/transform.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+
+#include <string>
+#include <unordered_map>
+
+#include "pattern_utils.h"
+
+namespace tvm {
+namespace relay {
+
+class DefuseOpsMutator : public ExprMutator {
+ public:
+  class FuncBodyMutator : public ExprMutator {
+   public:
+    explicit FuncBodyMutator(const Array<Expr>& args) : ExprMutator() { args_ = args; }
+
+    Expr VisitExpr_(const VarNode* n) {
+      const std::string& name = n->name_hint();
+      ICHECK_EQ(name[0], 'p');
+      std::string id_str = name.substr(1);
+      int id = atoi(id_str.c_str());
+      ICHECK(id >= 0 && size_t(id) < args_.size());
+      return args_[id];
+    }
+
+    Array<Expr> args_;

Review comment:
       Suggest
   ```cpp
   private:
       Array<Expr> args_;
   ```




----------------------------------------------------------------
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