Lunderberg commented on PR #16305: URL: https://github.com/apache/tvm/pull/16305#issuecomment-1875525580
> You are using VisitVarDef but are ignoring vars defined inside SeqExprs, so I take it that this will update only function parameters (per the name). Is that correct? That is correct. Only parameters would be updated, and variable bindings within the function body would have their struct info inferred. > I'm not sure if it would be simpler to override the function node case instead of VisitVarDef. I initially ran into some issues with overriding the `Expr VisitExpr_(const FunctionNode*)` mutator, mostly because it was difficult without copy-pasting the entire `ExprMutator` implementation. In order to delegate to the base class's mutator, it would need to first construct a `Function` object with updated parameters, and then pass that to the base class. However, that ran into issues with the validity-checking in the `Function` constructor, since the body of the function wasn't yet updated. Taking another look at it, it looks like it can be moved to the `VisitExpr_(const FunctionNode*)` by using `func.CopyOnWrite()` to avoid going through the normal constructor. I agree that this is simpler, and have updated the PR to use the less roundabout implementation. -- 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]
