[TIRx] Generalize expression functor signatures - #19931
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes the SizeVar and SizeVarNode classes, unifying symbolic variables under Var and VarNode. It also updates ExprFunctor and its subclasses to operate on Expr instead of PrimExpr to allow visiting and mutating general expressions. Additionally, BlockBuilder in Relax is updated to track active scopes and blocks to safely unwind them on exceptional exits. The reviewer identified a potential issue in _exit_function_scope where exceptions during block or scope finalization could skip subsequent cleanup steps, and suggested using a try...finally block to ensure robust cleanup.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
a4ba091 to
9d02d61
Compare
5c57f32 to
c232afb
Compare
Unify TIRX expression traversal on the shared Expr surface. Expose one generalized statement hook while keeping primitive reconstruction behind checked VisitPrimExpr boundaries.
c232afb to
b8e34e0
Compare
The Windows wheel build for `v0.26.0.rc0` fails to compile: ``` src\tirx\ir\data_type_rewriter.cc(696,47): error C2352: 'tvm::tirx::ExprMutator::VisitPrimExpr': a call of a non-static member function requires an object ``` `StmtExprMutator` derives from both `ExprMutator` and `StmtMutator`, and re-exports the name via `using ExprMutator::VisitPrimExpr;`. MSVC resolves the class-qualified `IndexDataTypeNormalizer::VisitPrimExpr` down to `ExprMutator::VisitPrimExpr` and then fails to form the implicit object conversion. GCC and Clang accept the same expression, so only the Windows leg broke — the macOS and both Linux wheels built fine. The fix calls it through `this` instead, matching every other `VisitPrimExpr` call site in this file. `VisitPrimExpr` is a non-virtual inline helper, so the qualification was suppressing nothing and behavior is unchanged. The other class-qualified call sites in the tree name `StmtExprMutator` directly — that is where the using-declaration lives, so they resolve fine and are left alone. The call was introduced in #19931, which changed `IndexDataTypeNormalizer::VisitExpr` to `IndexDataTypeNormalizer::VisitPrimExpr`; the former resolved unambiguously. Targeting the release branch first to unblock the `v0.26.0.rc0` Windows wheel; it will be ported to `main` separately. Failing job: https://github.com/apache/tvm/actions/runs/31033249253/job/92400739674
Port of #20096, which landed on the `v0.26.0` release branch first to unblock the `v0.26.0.rc0` Windows wheel. `main` has the same breakage. MSVC fails to compile the class-qualified call: ``` src\tirx\ir\data_type_rewriter.cc(696,47): error C2352: 'tvm::tirx::ExprMutator::VisitPrimExpr': a call of a non-static member function requires an object ``` `StmtExprMutator` derives from both `ExprMutator` and `StmtMutator` and re-exports the name via `using ExprMutator::VisitPrimExpr;`. MSVC resolves the qualified `IndexDataTypeNormalizer::VisitPrimExpr` down to `ExprMutator::VisitPrimExpr` and then fails to form the implicit object conversion. GCC and Clang accept the same expression, so only the Windows leg broke — macOS and both Linux wheels built fine. The fix calls it through `this`, matching every other `VisitPrimExpr` call site in this file. `VisitPrimExpr` is a non-virtual inline helper, so the qualification was suppressing nothing and behavior is unchanged. The other class-qualified call sites in the tree name `StmtExprMutator` directly — that is where the using-declaration lives, so they resolve fine and are left alone. The call was introduced in #19931, which changed `IndexDataTypeNormalizer::VisitExpr` to `IndexDataTypeNormalizer::VisitPrimExpr`; the former resolved unambiguously. Original failing job: https://github.com/apache/tvm/actions/runs/31033249253/job/92400739674
Expression unification gives TIRX a shared
Exprsurface, but its visitor and mutator APIs still expose primitive-only signatures. That mismatch prevents general expressions from flowing through the existing traversal structure and leaves statement traversal with overlapping customization hooks.This refactor generalizes the existing
ExprFunctor,ExprVisitor, andExprMutatorsignatures in place to accept and returnExpr. Statement visitors and mutators expose a single virtualVisitExpr(const Expr&)hook, while primitive statement reconstruction uses a non-virtual checkedVisitPrimExprhelper so invalid narrowing fails at the boundary. Public pre-order and post-order traversal entry points accept generalExprroots.The existing specialization, vtable, dispatch registration, and class structure remain intact; the change adds no parallel functor, fallback dispatcher, or alternate implementation path.