Lunderberg commented on code in PR #16595:
URL: https://github.com/apache/tvm/pull/16595#discussion_r1499420876
##########
include/tvm/relax/expr.h:
##########
@@ -169,13 +169,11 @@ class CallNode : public ExprNode {
bool SEqualReduce(const CallNode* other, SEqualReducer equal) const {
// skip sinfo_args check for primitive ops.
- equal->MarkGraphNode();
Review Comment:
The `MarkGraphNode` function should be called for IR nodes that have
reference equality, such as variables. Using it with the `relax::CallNode`
means that results from `StructuralEqual` will be dependent on how a
`relax::Call` node was constructed, even if they have identical contents.
Relevant to this PR, if `R.zeros([16], "int32")` appears in both `main` and
`transform_params`, the expected output generated by the TVMScript parser would
have two different `relax::Call` objects, while the output of
`LiftTransformParams` would use the same `relax::Call` object. Because the
`relax::Call` objects were being checked for analogous reference equality, this
would cause `StructuralEqual` to erroneously report the test as failing.
I've added a unit test to specifically exercise this behavior, rather than
implicitly relying on the tests for `LiftTransformParams`.
--
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-unsubscr...@tvm.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
