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