gemini-code-assist[bot] commented on PR #18874: URL: https://github.com/apache/tvm/pull/18874#issuecomment-3999907521
## Summary of Changes Hello, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant refactoring of the TIR (Tensor Intermediate Representation) `LetStmt` node, renaming it to `Bind` and altering its semantics to flatten nested bindings into sequential statements. This change simplifies the IR, improves readability, and facilitates easier analysis and transformation. The update includes migrating existing code to use the new `BindNode` and introducing a `ScopeStack` for managing variable scopes in passes that require it. ### Highlights * **Refactoring**: Renames `LetStmtNode`/`LetStmt` to `BindNode`/`Bind`, providing a more intuitive name for variable bindings. * **Semantic Change**: Removes the `body` field from the `BindNode`. The variable defined by `Bind(var, value)` is now visible in all subsequent statements within the same enclosing scope, rather than being scoped to a nested body. * **IR Simplification**: Flattens deeply nested let-chains into sequential `SeqStmt([Bind(...), Bind(...), ...])`, making the IR easier to read, transform, and analyze. * **Scope Management**: Introduces a `ScopeStack` pattern for passes that need scope-aware cleanup, providing clean RAII-based scope management. * **Migration**: Migrates all passes to use the new `BindNode` and `ScopeStack` pattern, updating 89 files across codegen backends, TIR transforms, S-TIR transforms, analyses, and TVMScript printer/parser/ir_builder. 🧠**New Feature in Public Preview:** You can now enable **Memory** to help **Gemini Code Assist** learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. **Click [here](https://codeassist.google/code-review/login) to enable Memory in your admin console.** <details> <summary><b>Changelog</b></summary> * **include/tvm/script/ir_builder/tir/frame.h** * Renamed `LetFrameNode` to `BindFrameNode` and `LetFrame` to `BindFrame` to reflect the change from LetStmt to Bind. * **include/tvm/script/ir_builder/tir/ir.h** * Renamed `LetStmt` function to `Bind` to align with the new BindNode. * **include/tvm/script/printer/ir_docsifier_functor.h** * Added `TVM_FFI_UNREACHABLE()` to handle unexpected object types during documentation generation. * **include/tvm/tir/stmt.h** * Renamed `LetStmtNode` to `BindNode` and removed the `body` field, changing the scoping semantics of variable bindings. * Added `TVM_FFI_UNREACHABLE()` to `ForKind2String` to handle unknown `ForKind` values. * **include/tvm/tir/stmt_functor.h** * Replaced `LetStmtNode` with `BindNode` in `StmtFunctor` and related classes to reflect the renaming. * **include/tvm/tir/var.h** * Updated documentation to replace `LetStmt` with `Bind` in the list of nodes that can define a variable. * **python/tvm/script/ir_builder/tir/frame.py** * Renamed `LetFrame` to `BindFrame` in the Python bindings. * **python/tvm/script/ir_builder/tir/ir.py** * Renamed `LetStmt` function to `Bind` and updated docstrings to reflect the change to Bind semantics. * Deprecated `LegacyLetStmt` and updated `let_stmt` to use `Bind`. * **python/tvm/script/parser/core/parser.py** * Added support for attribute assignment in duplicate LHS check. * **python/tvm/script/parser/tir/parser.py** * Replaced `T.LetStmt` with `T.Bind` in variable assignment and annotation parsing. * **python/tvm/tir/__init__.py** * Replaced `LetStmt` with `Bind` in the list of imported symbols. * **python/tvm/tir/functor.py** * Replaced `LetStmt` with `Bind` in the list of statement types and updated visitor functions. * **python/tvm/tir/stmt.py** * Renamed `LetStmt` class to `Bind` and updated its attributes and constructor to reflect the removal of the `body` field. * **python/tvm/tir/transform/transform.py** * Replaced `LetStmt` with `Bind` in the `HoistedLetBindings` enum. * **src/arith/ir_mutator_with_analyzer.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and removed handling of the `body` field. * **src/arith/ir_mutator_with_analyzer.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * **src/arith/ir_visitor_with_analyzer.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and removed visiting the `body` field. * **src/arith/ir_visitor_with_analyzer.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * **src/relax/op/tensor/inspect.cc** * Modified `GetDLTensorField` and `LegalizeTensorShape` to use `Bind` instead of `LetStmt` and to construct statements with `SeqStmt`. * **src/s_tir/analysis/estimate_flops.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and removed visiting the `body` field. * **src/s_tir/analysis/sblock_access_region_detector.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and adjusted how let bindings are handled. * **src/s_tir/backend/adreno/inject_texture_alloc.cc** * Replaced `LetStmt` with `SeqStmt` and `Bind` to allocate texture memory. * **src/s_tir/schedule/analysis/reducer.cc** * Replaced references to `LetStmt` with `Bind` and updated the logic for extracting reduction updates to accommodate the new Bind semantics. * Modified `ExtractReductionUpdates` to handle `Bind` nodes and sequences of statements instead of nested `LetStmt` nodes. * **src/s_tir/schedule/primitive/blockize_tensorize.cc** * Added `TVM_FFI_UNREACHABLE()` to handle unexpected target types in `ApplyToSchedule`. * **src/s_tir/schedule/primitive/layout_transformation.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode`. * **src/s_tir/schedule/primitive/reduction.cc** * Modified `BaseBlockCreator::CreateBlockBody` to use `Bind` nodes instead of `LetStmt` nodes for creating reduction blocks. * **src/s_tir/transform/compact_buffer_region.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and adjusted how domain analysis is performed. * **src/s_tir/transform/hoist_expression.cc** * Renamed `kLetStmt` to `kBind` in the `HoistedLetBindings` enum and updated related logic. * Modified `HoistInfoCollector` to handle `BindNode` and `SeqStmtNode` for expression hoisting. * **src/s_tir/transform/inject_virtual_thread.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and updated the logic for injecting virtual thread loops. * Added `TVM_FFI_UNREACHABLE()` to `VisitStmt_` for `WhileNode` to indicate that WhileNode is not supported. * **src/s_tir/transform/lower_thread_allreduce.cc** * Modified `ThreadAllreduceBuilder` to use `Bind` instead of `LetStmt` for creating reduction statements. * **src/s_tir/transform/lower_vtcm_alloc.cc** * Replaced `LetStmt` with `SeqStmt` and `Bind` in `VtcmAllocator`. * **src/s_tir/transform/profile_instrumentation.cc** * Modified `LoopAnalyzer::TraverseLoop` to handle `BindNode` and `SeqStmtNode` for loop analysis. * **src/s_tir/transform/remove_store_undef.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and updated the logic for locating and removing store undef statements. * Modified `StoreUndefLocator` to return `UndefInfo` containing both undef stores and undef bind vars. * Modified `StoreUndefRemover` to remove `Bind` nodes with undef values. * **src/s_tir/transform/renew_defs.cc** * Replaced `LetStmtNode` with `BindNode` in `STMT_REGENERATE_VAR_DEF` macro. * **src/s_tir/transform/storage_access.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and updated the logic for storage access analysis. * **src/s_tir/transform/storage_access.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * **src/script/ir_builder/tir/frame.cc** * Replaced `LetFrameNode` with `BindFrameNode` and updated the logic for exiting with scope. * Added `TVM_FFI_STATIC_INIT_BLOCK` to register `BindFrameNode`. * **src/script/ir_builder/tir/ir.cc** * Replaced `LetStmt` with `Bind` and updated related functions and registration. * Removed `LegacyLetStmt` function. * **src/script/printer/relax/distributed.cc** * Added `TVM_FFI_UNREACHABLE()` to handle internal errors during documentation generation. * **src/script/printer/tir/stmt.cc** * Replaced `tir::LetStmt` with `tir::Bind` in the IRDocsifier and updated the logic for printing statements. * Added `TVM_FFI_UNREACHABLE()` to handle unimplemented fragment printing. * **src/target/llvm/codegen_llvm.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and removed handling of the `body` field. * **src/target/llvm/codegen_llvm.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * **src/target/source/codegen_c.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and removed handling of the `body` field. * Added `TVM_FFI_UNREACHABLE()` to handle unsupported type indices. * **src/target/source/codegen_c.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * **src/target/source/codegen_webgpu.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and removed handling of the `body` field. * **src/target/source/codegen_webgpu.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * **src/target/spirv/codegen_spirv.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and removed handling of the `body` field. * **src/target/spirv/codegen_spirv.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * **src/te/operation/create_primfunc.cc** * Replaced `LetStmt` with `Bind` and updated the logic for generating body statements in `GenerateBodyStmt`. * **src/tir/analysis/control_flow_graph.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode`. * **src/tir/analysis/var_use_def_analysis.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode`. * **src/tir/analysis/var_use_def_analysis.h** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode`. * **src/tir/analysis/verify_memory.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode`. * **src/tir/analysis/verify_ssa.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode`. * **src/tir/ir/data_type_rewriter.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and updated the logic for data type legalization. * Modified `IndexDataTypeRewriter::VisitStmt_` to handle `Bind` nodes and update variable remapping. * **src/tir/ir/data_type_rewriter.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * **src/tir/ir/py_functor.cc** * Replaced `f_visit_let_stmt` with `f_visit_bind` and updated related dispatch logic. * Replaced `PY_STMT_VISITOR_DEFAULT_DISPATCH(LetStmtNode)` with `PY_STMT_VISITOR_DEFAULT_DISPATCH(BindNode)`. * **src/tir/ir/specialize.cc** * Updated comments to refer to `Bind` instead of `LetStmt`; modified `PrimFuncSpecializer` to use `SeqStmt` and `Bind` for remapping buffer variables. * **src/tir/ir/stmt.cc** * Replaced `LetStmtNode` with `BindNode` and updated the constructor and reflection registration. * Removed `LetStmt` and updated `Bind` to reflect the new semantics. * **src/tir/ir/stmt_functor.cc** * Replaced `VisitStmt_` for `LetStmtNode` with `VisitStmt_` for `BindNode` and updated the logic for visiting statements. * **src/tir/ir/tir_visitor_with_path.cc** * Replaced `VisitStmt_` for `LetStmtNode` with `VisitStmt_` for `BindNode` and updated the logic for visiting statements with access paths. * Modified `TIRVisitorWithPath::Visit` to use `ScopeStack` for managing variable scopes. * **src/tir/ir/tir_visitor_with_path.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * Added `ScopeStack` for managing variable scopes. * **src/tir/transform/common_subexpr_elim.cc** * Replaced `VisitStmt_` for `LetStmtNode` with `VisitStmt_` for `BindNode` and updated the logic for common subexpression elimination. * Modified `CommonSubexpressionEliminator` to use `ScopeStack` for managing variable scopes and context cleanup. * Modified `CommonSubexpressionEliminator::VisitStmt_` to handle `SeqStmtNode` and `BindNode` for CSE. * **src/tir/transform/common_subexpr_elim.h** * Replaced `LetStmtNode` with `BindNode` in the `VisitStmt_` override. * Added `ScopeStack` for managing variable scopes and context cleanup. * **src/tir/transform/ir_utils.cc** * Replaced `LetStmtNode` with `BindNode` in `MergeNest` function. * Modified `IRConvertSSA` to use `ScopeStack` for managing variable scopes and update variable remapping. * **src/tir/transform/ir_utils.h** * Replaced `LetStmtNode` with `BindNode` in the description of `MergeNest`. * **src/tir/transform/lower_tvm_builtin.cc** * Modified `BuiltinLower` to use `Bind` and `SeqStmt` for stack allocation and `nd_mem_alloc_with_scope`. * **src/tir/transform/remove_no_op.cc** * Modified `NoOpRemover` to preserve `Bind` nodes, deferring unused `Bind` elimination to a separate pass. * **src/tir/transform/simplify.cc** * Removed `CollectVarsUsedInBufferDefinition` and `used_in_buffer_def_` from `StmtSimplifier`; modified `VisitStmt_` for `BindNode` to always retain the `Bind` statement. * **src/tir/transform/split_host_device.cc** * Replaced `LetStmt` with `SeqStmt` and `Bind` for kernel call checks. * **src/tir/transform/storage_rewrite.cc** * Modified `LinearAccessPatternFinder` to process `BindNode` and `VectorTypeRewriter` to handle `BindNode`. * **src/tir/transform/tvm_ffi_binder.cc** * Replaced `LetStmt` with `Bind` in `TVMFFIABIBuilder` for scalar binding and DLTensor field pointers. * **src/tir/transform/tvm_ffi_binder.h** * Updated comments to refer to `Binds` instead of `LetStmts` in `TVMFFIABIBuilder` documentation. * **src/tir/transform/unsupported_dtype_legalize.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and updated the logic for data type promotion. * **src/tir/transform/vectorize_loop.cc** * Renamed `VisitStmt_` for `LetStmtNode` to `VisitStmt_` for `BindNode` and updated the logic for vectorization. * **tests/python/s_tir/schedule/test_tir_schedule_rfactor.py** * Renamed test functions and references from `letstmt` to `bind`. * **tests/python/s_tir/transform/test_s_tir_transform_hoist_expression.py** * Renamed test functions and references from `LetStmt` to `Bind`; updated expected output in `test_hoist_disable_let`. * **tests/python/s_tir/transform/test_s_tir_transform_inject_ptx_async_copy.py** * Updated test to reflect changes in CSE variable names. * **tests/python/s_tir/transform/test_s_tir_transform_plan_update_buffer_allocation_location.py** * Updated test description to reflect the change from LetStmt to Bind. * **tests/python/s_tir/transform/test_s_tir_transform_thread_sync.py** * Renamed test functions and references from `let_stmt` to `bind`. * **tests/python/tir-analysis/test_tir_analysis_verify_ssa.py** * Updated test to use `tir.SeqStmt` and `tir.Bind` instead of `tir.LetStmt`. * **tests/python/tir-analysis/test_tir_analysis_verify_well_formed.py** * Updated test descriptions and code to reflect the change from LetStmt to Bind; modified tests to account for flat Bind semantics and scope management. * **tests/python/tir-base/test_tir_constructor.py** * Updated test to use `tir.Bind` instead of `tir.LetStmt`. * **tests/python/tir-base/test_tir_nodes.py** * Updated test to use `tir.Bind` instead of `tir.LetStmt`. * **tests/python/tir-base/test_tir_structural_equal_hash.py** * Updated test to use `tir.SeqStmt` and `tir.Bind` instead of `tir.LetStmt`. * **tests/python/tir-transform/test_tir_inline_private_functions.py** * Replaced `T.LetStmt` with `T.Bind` in the expected output. * **tests/python/tir-transform/test_tir_transform_common_subexpr_elim.py** * Updated test to use `tir.SeqStmt` and `tir.Bind` instead of `tir.LetStmt` and adjusted assertions for flat IR. * **tests/python/tir-transform/test_tir_transform_convert_ssa.py** * Renamed test functions and updated code to use `tir.Bind` and `tir.SeqStmt` instead of `tir.LetStmt`. * **tests/python/tir-transform/test_tir_transform_lower_tvm_builtin.py** * Updated test to use `tir.Bind` and `tir.SeqStmt` instead of `tir.LetStmt`. * **tests/python/tir-transform/test_tir_transform_prim_func_pass.py** * Updated test to use `tir.SeqStmt` and `tir.Bind` instead of `tir.LetStmt`. * **tests/python/tir-transform/test_tir_transform_remove_no_op.py** * Updated test descriptions and code to reflect `Bind` semantics, noting that unused `Bind` nodes are now preserved. * **tests/python/tir-transform/test_tir_transform_simplify.py** * Updated test descriptions and code to reflect `Bind` semantics and the behavior of the simplifier with flat Bindings. * **tests/python/tir-transform/test_tir_transform_storage_rewrite.py** * Updated test description to reflect the change from `LetStmt` to `Bind`. * **tests/python/tvmscript/test_tvmscript_ir_builder_tir.py** * Updated test to use `T.Bind` instead of `T.LetStmt` and to check for a `tir.SeqStmt`. * **tests/python/tvmscript/test_tvmscript_printer_annotation.py** * Updated test to reflect changes in the generated IR structure due to the change from LetStmt to Bind. * **tests/python/tvmscript/test_tvmscript_printer_tir.py** * Updated test to use `T.Bind` instead of `T.LetStmt` and to check for a `tir.SeqStmt`. * **tests/python/tvmscript/test_tvmscript_roundtrip.py** * Removed `opt_gemm_mod_host` from the ir_generator parameter; updated tests to reflect changes in the generated IR structure due to the change from LetStmt to Bind. * **tests/python/tvmscript/test_tvmscript_syntax_sugar.py** * Renamed test functions and updated code to use `T.Bind` instead of `T.LetStmt`. </details> <details> <summary><b>Activity</b></summary> * The pull request involves renaming `LetStmtNode`/`LetStmt` to `BindNode`/`Bind` and removing the `body` field. * The variable defined by `Bind(var, value)` is now visible in all subsequent statements within the same enclosing scope. * Deeply nested let-chains are flattened into sequential `SeqStmt([Bind(...), Bind(...), ...])`. * 89 files were updated across codegen backends, TIR transforms, S-TIR transforms, analyses, and TVMScript printer/parser/ir_builder. * ScopeStack pattern is used for scope-aware cleanup. </details> <details> <summary><b>Using Gemini Code Assist</b></summary> <br> The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips. <b>Invoking Gemini</b> You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page. Feature | Command | Description --- | --- | --- Code Review | `/gemini review` | Performs a code review for the current pull request in its current state. Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state. Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. Help | `/gemini help` | Displays a list of available commands. <b>Customization</b> To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). <b>Limitations & Feedback</b> Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG). <b>You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the [Gemini Code Assist IDE Extension](https://cloud.google.com/products/gemini/code-assist).</b> </details> [^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487). -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
