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 2021/12/02 17:24:52 UTC
[GitHub] [tvm] mbs-octoml commented on a change in pull request #9628: [CORE][MVP] POC of fuse ops pass based on the DFPatterns
mbs-octoml commented on a change in pull request #9628:
URL: https://github.com/apache/tvm/pull/9628#discussion_r761302988
##########
File path: include/tvm/relay/dataflow_matcher.h
##########
@@ -97,15 +97,29 @@ Expr RewritePatterns(Array<DFPatternCallback> callbacks, Expr expr, IRModule mod
* \brief Partition all matches of a DFPattern inside an Expr into separate Function calls
*
* \param pattern The pattern to match
- * \param expr The expression to patition
+ * \param expr The expression to partition
* \param attrs A set of parameter names and values to apply to the partitioned function
* \param check A callback function for checking more complicated properties of the matched
* expressions, returns true if the match is accepted and false otherwise
*
- * \return Return the paritioned Expr.
+ * \return Return the partitioned Expr.
*/
Expr PartitionPattern(DFPattern pattern, Expr expr, Map<String, ObjectRef> attrs, PackedFunc check);
+/*!
+ * \brief Partition all matches of a DFPattern inside an Expr into separate Function calls
Review comment:
You'll need to explain the 'hierarchical order' part here, perhaps explain they are expected to be in most-specific to most-general form and the first pattern to succeed is taken.
##########
File path: src/relay/ir/dataflow_matcher.cc
##########
@@ -867,10 +867,18 @@ class PatternPartitioner : protected MixedModeMutator {
return Call(func, args);
}
+ // Expr DispatchVisitExpr(const Expr& pre) override {
Review comment:
nit: nuke i guess
##########
File path: include/tvm/relay/transform.h
##########
@@ -250,14 +258,24 @@ TVM_DLL Pass DynamicToStatic();
/*!
* \brief Infer the type of an expression.
*
- * The result of type checking is a new expression with unambigous
+ * The result of type checking is a new expression with unambiguous
* type information filled in, as well as it's checked type field
* populated with the result type.
*
* \return The pass.
*/
TVM_DLL Pass InferType();
+/*!
+ * \brief Annoate primitive functions
+ *
+ * The result is an update module with annotated the primitive functions originated from the fuse
Review comment:
nit: ...updated module with fused functions annotation...
##########
File path: src/relay/transforms/pattern_fuse.cc
##########
@@ -0,0 +1,306 @@
+/*
+ * 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/fold_explicit_padding.cc
Review comment:
nit: update
##########
File path: src/relay/ir/dataflow_matcher.cc
##########
@@ -887,6 +895,15 @@ Expr PartitionPattern(DFPattern pattern, Expr expr, Map<String, ObjectRef> attrs
return PatternPartitioner().Partition(pattern, expr, attrs, check);
}
+Expr PartitionPattern(Array<DFPattern> patterns, Expr expr, Map<String, ObjectRef> attrs,
Review comment:
I'm not sure, but perhaps this is better expressed as a 'sequence' pattern combinator who's matching rule is what you've written here.
##########
File path: src/relay/ir/dataflow_matcher.cc
##########
@@ -867,10 +867,18 @@ class PatternPartitioner : protected MixedModeMutator {
return Call(func, args);
}
+ // Expr DispatchVisitExpr(const Expr& pre) override {
+ // auto post = MixedModeMutator::DispatchVisitExpr(pre);
+ // if (gid_assignments_.count(pre) && pre == groups_[gid_assignments_[pre]].root_node &&
+ // static_cast<bool>(check_(pre))) {
+ // post = RewritePartition(groups_[gid_assignments_[pre]]);
+ // }
+ // return post;
+ // }
+
Expr DispatchVisitExpr(const Expr& pre) override {
auto post = MixedModeMutator::DispatchVisitExpr(pre);
- if (gid_assignments_.count(pre) && pre == groups_[gid_assignments_[pre]].root_node &&
- static_cast<bool>(check_(pre))) {
+ if (gid_assignments_.count(pre) && pre == groups_[gid_assignments_[pre]].root_node) {
Review comment:
sorry i don't understand this change
##########
File path: src/relay/transforms/annotate_post_fuse_funcs.cc
##########
@@ -0,0 +1,79 @@
+/*
+ * 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/fold_explicit_padding.cc
Review comment:
nit: update
##########
File path: src/relay/transforms/pattern_fuse.cc
##########
@@ -0,0 +1,306 @@
+/*
+ * 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/fold_explicit_padding.cc
Review comment:
could you comment throughout here?
##########
File path: src/relay/transforms/annotate_post_fuse_funcs.cc
##########
@@ -0,0 +1,79 @@
+/*
+ * 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/fold_explicit_padding.cc
+ * \brief A pass for folding explicit pads into other ops.
+ */
+
+#include <tvm/relay/dataflow_matcher.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+#include <tvm/runtime/logging.h>
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "../op/tensor/transform.h"
+#include "./pattern_fuse.h"
+
+namespace tvm {
+namespace relay {
+
+namespace transform {
+
+Pass AnnotatePostFuseFuncs() {
+ auto pass_info = PassInfo(0, "AnnotatePostFuseFuncs", {});
+ return tvm::transform::CreateModulePass(
+ [=](IRModule mod, const PassContext& pass_ctx) {
+ // Execute the pass function and return a new module.
+ IRModule updated_mod = mod->ShallowCopy();
+
+ pass_ctx->diag_ctx = DiagnosticContext::Default(updated_mod);
+
+ std::vector<std::pair<GlobalVar, Function> > updates;
+ for (const auto& it : updated_mod->functions) {
+ if (auto* func_node = it.second.as<FunctionNode>()) {
+ auto func = GetRef<Function>(func_node);
+
+ // add check from where it originate
+ func = WithAttr(std::move(func), attr::kPrimitive, tvm::Integer(1));
Review comment:
this will annotate all functions, including used defined ones.
meanwhile isn't fusion rewriting the sub-expression to a call to a function literal, and it's those that need the primitive annotation?
##########
File path: src/relay/transforms/annotate_post_fuse_funcs.cc
##########
@@ -0,0 +1,79 @@
+/*
+ * 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/fold_explicit_padding.cc
+ * \brief A pass for folding explicit pads into other ops.
+ */
+
+#include <tvm/relay/dataflow_matcher.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+#include <tvm/relay/transform.h>
+#include <tvm/runtime/logging.h>
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "../op/tensor/transform.h"
+#include "./pattern_fuse.h"
+
+namespace tvm {
+namespace relay {
+
+namespace transform {
+
+Pass AnnotatePostFuseFuncs() {
+ auto pass_info = PassInfo(0, "AnnotatePostFuseFuncs", {});
+ return tvm::transform::CreateModulePass(
Review comment:
this can be a FunctionPass right?
##########
File path: include/tvm/relay/dataflow_matcher.h
##########
@@ -97,15 +97,29 @@ Expr RewritePatterns(Array<DFPatternCallback> callbacks, Expr expr, IRModule mod
* \brief Partition all matches of a DFPattern inside an Expr into separate Function calls
*
* \param pattern The pattern to match
- * \param expr The expression to patition
+ * \param expr The expression to partition
* \param attrs A set of parameter names and values to apply to the partitioned function
* \param check A callback function for checking more complicated properties of the matched
* expressions, returns true if the match is accepted and false otherwise
*
- * \return Return the paritioned Expr.
+ * \return Return the partitioned Expr.
*/
Expr PartitionPattern(DFPattern pattern, Expr expr, Map<String, ObjectRef> attrs, PackedFunc check);
+/*!
+ * \brief Partition all matches of a DFPattern inside an Expr into separate Function calls
Review comment:
Oh now that i look at the impl i see it's not that at all. So yeah will need to explain :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org