Lunderberg commented on issue #17210: URL: https://github.com/apache/tvm/issues/17210#issuecomment-2256389226
Ooh, that ended up being a real tricky one. There was an implicit assumption in my first implementation that every group would contain distinct objects. This is usually true, because each `relax::Var` must be unique across an `IRModule`, and so the expressions containing `relax::Var` must also be unique in each function. However, relax expressions do not depend on any variables, such as static shapes, may re-use the same underlying C++ object across multiple functions. In the test case, both `main` and `main2` inferred the return type of `fused_relax_nn_conv2d_relax_nn_relu`, using the same `ShapeExpr`. This was then assigned to its own group during a `PostOrderVisit`, but that group was never actually used. For now, I've added a fix to avoid over-collection of `ShapeExpr`, by removing the use of `PostOrderVisit`. The bug could still be triggered for shape expressions that occur explicitly within `relax::Call` arguments (e.g. for `R.full` arguments), and which are re-used across multiple functions in the same `IRModule`. That should be a rarer case, and since it will be a larger refactor to avoid those edge cases (to have caching on a per-`Var` basis, rather than a per-`const Object*` basis), I think I'll wait until that becomes an issue before tackling it. -- 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]
