Lunderberg commented on code in PR #17033:
URL: https://github.com/apache/tvm/pull/17033#discussion_r1617284952
##########
src/relax/transform/fuse_ops.cc:
##########
@@ -262,16 +262,11 @@ class GraphCreator : public ExprVisitor {
IndexedForwardGraph::Node* leaf_node = nullptr;
if (it != graph_.node_map.end()) {
leaf_node = it->second;
- } else if (leaf_expr->IsInstance<ConstantNode>() ||
leaf_expr->IsInstance<ShapeExprNode>() ||
- leaf_expr->IsInstance<PrimValueNode>() ||
leaf_expr->IsInstance<StringImmNode>() ||
- leaf_expr->IsInstance<DataTypeImmNode>()) {
+ } else {
Review Comment:
This resolves an issue that results from the use of `PostOrderVisit`
[here](https://github.com/apache/tvm/blob/main/src/relax/transform/fuse_ops.cc#L230),
in `VisitUnsupportedNode`. It looks like this function is intended to collect
all `Var`/`Constant` nodes that are part of the RHS of a binding (e.g.
extracting `A` and `B` from `C = R.add(A,B)`). However, it doesn't collect any
bindings that may be part of a nested expression. As a result, variable
bindings that are part of a conditional's body would erroneously trigger the
`LOG(FATAL)` in `VisitLeaf`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
