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/11/10 16:27:14 UTC

[GitHub] [tvm] mbs-octoml commented on a change in pull request #9479: Extend op attrs with commutattive annotation

mbs-octoml commented on a change in pull request #9479:
URL: https://github.com/apache/tvm/pull/9479#discussion_r746760352



##########
File path: src/relay/op/op_common.h
##########
@@ -84,6 +85,7 @@ namespace relay {
       .add_type_rel("Broadcast", BroadcastRel)                                          \
       .set_attr<TOpPattern>("TOpPattern", kBroadcast)                                   \
       .set_attr<TOpIsStateful>("TOpIsStateful", false)                                  \
+      .set_attr<CommutativeOp>("CommutativeOp", isCommutativeOp(OpName))                \

Review comment:
       What if it's non-comm by default (so no attrib binding), and the registration of (for now) add & multiply explicitly set it to true.

##########
File path: src/relay/op/op_common.h
##########
@@ -60,6 +60,7 @@ namespace relay {
       .add_type_rel("Identity", IdentityRel)                                   \
       .set_attr<TOpPattern>("TOpPattern", kElemWise)                           \
       .set_attr<TOpIsStateful>("TOpIsStateful", false)                         \
+      .set_attr<CommutativeOp>("CommutativeOp", false)                         \

Review comment:
       well, I guess so...

##########
File path: src/relay/ir/dataflow_matcher.cc
##########
@@ -213,6 +213,14 @@ bool DFPatternMatcher::VisitDFPattern_(const CallPatternNode* op, const Expr& ex
     }
     return false;
   };
+  auto is_commutative_op = [&get_op_node](const CallPatternNode* op_node) {

Review comment:
       I was expecting this to look at the TCommutativeOp attribute of op_node.

##########
File path: include/tvm/relay/op_attr_types.h
##########
@@ -89,6 +89,11 @@ using TNonComputational = bool;
  */
 using TReshapeOp = bool;
 
+/*!
+ * \brief Mark the operator as commutative to allow expr simplifications.
+ */
+using CommutativeOp = bool;

Review comment:
       nit: TCommutativeOp




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