You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "vinx13 (via GitHub)" <gi...@apache.org> on 2023/02/24 02:02:28 UTC

[GitHub] [tvm] vinx13 opened a new pull request, #14109: [Unity] Add callback to FuseOpsByPattern to check match result is accepted

vinx13 opened a new pull request, #14109:
URL: https://github.com/apache/tvm/pull/14109

   This adds a callback check function to `FuseOpsByPattern` to further check whether the match result should be accepted.
   
   cc @yelite @masahi 


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

[GitHub] [tvm] vinx13 commented on a diff in pull request #14109: [Unity] Add callback to FuseOpsByPattern to check match result is accepted

Posted by "vinx13 (via GitHub)" <gi...@apache.org>.
vinx13 commented on code in PR #14109:
URL: https://github.com/apache/tvm/pull/14109#discussion_r1117453966


##########
src/relax/transform/fuse_ops.cc:
##########
@@ -908,23 +908,27 @@ class PatternBasedPartitioner : ExprVisitor {
   using GroupMap = OperatorFusor::GroupMap;
   using ExprVisitor::VisitExpr_;
 
-  static GroupMap Run(String pattern_name, DFPattern pattern, Expr expr, support::Arena* arena) {
-    PatternBasedPartitioner part(pattern_name, pattern, AnalyzeVar2Value(expr));
+  static GroupMap Run(String pattern_name, DFPattern pattern, runtime::PackedFunc check, Expr expr,
+                      support::Arena* arena) {
+    PatternBasedPartitioner part(pattern_name, pattern, check, AnalyzeVar2Value(expr));
     // Initialize each expr to have its own group
     PostOrderVisit(
         expr, [arena, &part](const Expr& e) { part.group_map_[e.get()] = arena->make<Group>(); });
     part.VisitExpr(expr);
     return part.group_map_;
   }
 
-  PatternBasedPartitioner(String pattern_name, DFPattern pattern, const Map<Var, Expr>& bindings)
+  PatternBasedPartitioner(String pattern_name, DFPattern pattern, runtime::PackedFunc check,

Review Comment:
   Updated, there are still a few places using `Array<PackedFunc>` because `TypedPackedFunc` can't be Array element



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

[GitHub] [tvm] yelite commented on a diff in pull request #14109: [Unity] Add callback to FuseOpsByPattern to check match result is accepted

Posted by "yelite (via GitHub)" <gi...@apache.org>.
yelite commented on code in PR #14109:
URL: https://github.com/apache/tvm/pull/14109#discussion_r1116497037


##########
src/relax/transform/fuse_ops.cc:
##########
@@ -908,23 +908,27 @@ class PatternBasedPartitioner : ExprVisitor {
   using GroupMap = OperatorFusor::GroupMap;
   using ExprVisitor::VisitExpr_;
 
-  static GroupMap Run(String pattern_name, DFPattern pattern, Expr expr, support::Arena* arena) {
-    PatternBasedPartitioner part(pattern_name, pattern, AnalyzeVar2Value(expr));
+  static GroupMap Run(String pattern_name, DFPattern pattern, runtime::PackedFunc check, Expr expr,
+                      support::Arena* arena) {
+    PatternBasedPartitioner part(pattern_name, pattern, check, AnalyzeVar2Value(expr));
     // Initialize each expr to have its own group
     PostOrderVisit(
         expr, [arena, &part](const Expr& e) { part.group_map_[e.get()] = arena->make<Group>(); });
     part.VisitExpr(expr);
     return part.group_map_;
   }
 
-  PatternBasedPartitioner(String pattern_name, DFPattern pattern, const Map<Var, Expr>& bindings)
+  PatternBasedPartitioner(String pattern_name, DFPattern pattern, runtime::PackedFunc check,

Review Comment:
   Will it be better to use stronger typing, replacing `PackedFunc` with `TypedPackedFunc`, so that it's clearer on what to pass here?



##########
include/tvm/relax/transform.h:
##########
@@ -223,6 +223,8 @@ TVM_DLL Pass FuseOps(int fuse_opt_level = -1);
  * of a fused function after successful matching.
  * \param patterns The patterns to detect. The order of the patterns determines the order
  * of priority in which they are matched. Higher-priority patterns should come earlier in the list.
+ * \param checks The callback functions that takes a match result and returns a boolean value to

Review Comment:
   Taking a match result would work for simple cases, but will it still work for more complicated ones like MHA? I think it would be difficult to retrieve useful information from the match result alone, if the expr contains multiple nodes with similar nature. OTOH, It seems that only the call expr and the match result are available at the call site of checks function.



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

[GitHub] [tvm] masahi commented on pull request #14109: [Unity] Add callback to FuseOpsByPattern to check match result is accepted

Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi commented on PR #14109:
URL: https://github.com/apache/tvm/pull/14109#issuecomment-1447028427

   Need to resolve conflicts with https://github.com/apache/tvm/pull/14139 


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

[GitHub] [tvm] tvm-bot commented on pull request #14109: [Unity] Add callback to FuseOpsByPattern to check match result is accepted

Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #14109:
URL: https://github.com/apache/tvm/pull/14109#issuecomment-1442689677

   <!---bot-comment-->
   
   Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
   
   <!--bot-comment-ccs-start-->
    * cc @quic-sanirudh <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
   
   <sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>


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

[GitHub] [tvm] masahi commented on a diff in pull request #14109: [Unity] Add callback to FuseOpsByPattern to check match result is accepted

Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi commented on code in PR #14109:
URL: https://github.com/apache/tvm/pull/14109#discussion_r1116704997


##########
include/tvm/relax/transform.h:
##########
@@ -223,6 +223,8 @@ TVM_DLL Pass FuseOps(int fuse_opt_level = -1);
  * of a fused function after successful matching.
  * \param patterns The patterns to detect. The order of the patterns determines the order
  * of priority in which they are matched. Higher-priority patterns should come earlier in the list.
+ * \param checks The callback functions that takes a match result and returns a boolean value to

Review Comment:
   Yeah, I think it's useful to pass `CallNode` as well. We can filter out matches based on call node attributes etc.



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

[GitHub] [tvm] vinx13 commented on pull request #14109: [Unity] Add callback to FuseOpsByPattern to check match result is accepted

Posted by "vinx13 (via GitHub)" <gi...@apache.org>.
vinx13 commented on PR #14109:
URL: https://github.com/apache/tvm/pull/14109#issuecomment-1446865952

   Updated to make check func accept both match mapping and `Call`


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

[GitHub] [tvm] masahi merged pull request #14109: [Unity] Add callback to FuseOpsByPattern to check match result is accepted

Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi merged PR #14109:
URL: https://github.com/apache/tvm/pull/14109


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