[FFI] Bump tvm-ffi to 63224e3 and fix regressions#18938
[FFI] Bump tvm-ffi to 63224e3 and fix regressions#18938tqchen wants to merge 5 commits intoapache:mainfrom
Conversation
Update tvm-ffi submodule from c85fd42 (apache#471) to 63224e3 (apache#512). Fix two regressions from the bump: - Add no-op __init__ to relax.TEPlaceholderOp to prevent duplicate parameter name error from auto-init wiring (apache#491) - Use query_imports=True in Executable.__call__ to find main in imported sub-modules
Summary of ChangesHello, I'm Gemini Code Assist1! 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 focuses on updating the Highlights
🧠 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 to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the tvm-ffi subproject and modifies the ReprPrint functionality to use the TVMScript printer for ObjectRef values, both in Python and C++. It also includes minor refactorings in python/tvm/relax/expr.py, python/tvm/runtime/executable.py, and python/tvm/tirx/build.py. A review comment points out that the direct monkey-patching of tvm_ffi._ffi_api.ReprPrint in python/tvm/base.py is fragile and suggests a more robust public API in tvm-ffi for future improvements.
- Add Node.__repr__ to use TVMScript printer instead of dataclass repr - Fix host/device split: use target.kind.name instead of str(target.kind) - Override Module.__getattr__ to use query_imports=True
- Remove duplicate field declarations from C++ child types
(RXPlaceholderOpNode, FunctionFrameNode) so auto-init works correctly
- Add __slots__ = ("__dict__",) to Pass, BlockBuilder, and
TVMDerivedObject to support instance attributes under slots enforcement
- Revert unnecessary Module.__getattr__ override with query_imports=True
New tvm-ffi uses ffi.ReprPrint (dataclass repr) for CObject.__repr__ instead of TVM's ReprPrinter. This caused: - Target: str(target) returned dataclass repr instead of JSON - AccessPath: structural equality error messages showed verbose repr - PassInfo: pass name format changed in instrument output - ExprStmtDoc: __dict__ not available with __slots__ enforcement Fix by registering __ffi_repr__ TypeAttr for Target (returns JSON via TargetNode::str()), AccessPath and AccessStep (returns concise path format). Update tests for new PassInfo format and __slots__.
Summary
Bump tvm-ffi submodule from c85fd42 (#471) to 63224e3 (#512), spanning 41 commits with 7 breaking changes. Fix regressions introduced by the bump:
Fixes
Duplicate field declarations in C++ types — New tvm-ffi auto-wires
__init__from C++ reflection by walking the parent type chain. Child types that re-declared parent fields (RXPlaceholderOpNode,FunctionFrameNode) caused duplicate parameter errors. Fixed by removing duplicate field registrations from child types.Repr format regression (7 tests) — New tvm-ffi
CObject.__repr__uses dataclass repr. AddedNode.__repr__inpython/tvm/ir/base.pyto use TVMScript printer for IR nodes.Host/device function split (3 tests) —
str(target.kind)now returns full dataclass repr instead of kind name. Changed totarget.kind.nameinpython/tvm/tirx/build.py.__slots__enforcement — New tvm-ffi enforces__slots__=()on Object subclasses. Added__slots__ = ("__dict__",)to classes that need instance attributes:Pass,BlockBuilder,TVMDerivedObject.Changes
3rdparty/tvm-ffi— submodule bump c85fd42 → 63224e3python/tvm/ir/base.py—Node.__repr__using TVMScript printerpython/tvm/ir/transform.py—Pass.__slots__ = ("__dict__",)python/tvm/tirx/build.py—target.kind.nameinstead ofstr(target.kind)python/tvm/relax/block_builder.py—BlockBuilder.__slots__ = ("__dict__",)python/tvm/runtime/support.py—TVMDerivedObject.__slots__ = ("__dict__", "__weakref__")python/tvm/s_tir/meta_schedule/utils.py—TVMDerivedObject.__slots__ = ("__dict__",)include/tvm/script/ir_builder/relax/frame.h— remove duplicate field registrationssrc/relax/ir/emit_te.h— remove duplicate field registrationsTest plan