[REFACTOR][IR][FFI] Bump tvm-ffi (+ SEqHashDef migration) and phase out tvm/ir/repr.h#19627
Merged
spectrometerHBH merged 2 commits intoMay 27, 2026
Merged
Conversation
Bump 3rdparty/tvm-ffi from 3c35034 to 98d0029 (13 commits). The bump range brings in the new tvm/ffi/extra/dataclass.h ostream operator<< overloads (PR apache#596) used by the follow-up repr.h phase-out commit, plus the SEqHashDef -> SEqHashDef{Recursive,NonRecursive} rename (PR apache#583). All 21 in-tree SEqHashDef() call sites migrate to SEqHashDefRecursive() — the conservative variant matching the prior default behavior. Sites that are semantically let-style and should eventually flip to SEqHashDefNonRecursive once the new tvm-ffi releases on pypi carry a TODO(tqchen) comment: include/tvm/relax/expr.h:577 BindingNode::var include/tvm/relax/expr.h:619 MatchCastNode::struct_info include/tvm/tirx/buffer.h:129-135 BufferNode::data/shape/strides/elem_offset include/tvm/tirx/expr.h:701 LetNode::var include/tvm/tirx/stmt.h:89 BindNode::var include/tvm/tirx/stmt.h:276 AllocBufferNode::buffer
…class.h The previous commit bumped tvm-ffi to a version that ships ostream operator<< for Any / ObjectRef / Variant / Optional directly in tvm/ffi/extra/dataclass.h, making the in-tree tvm/ir/repr.h thin wrapper redundant. - Delete include/tvm/ir/repr.h. - Rewrite 8 includers: replace #include <tvm/ir/repr.h> with #include <tvm/ffi/extra/dataclass.h>, or just drop the include where dataclass.h was already pulled in transitively. - Rename src/ir/repr.cc to src/ir/access_path_repr.cc. The file's only remaining role is the node.AsRepr FFI registration plus the __ffi_repr__ hooks for ffi::reflection::AccessPath / AccessStep. Drop the (unused, zero in-tree callers) tvm::Dump() bodies; gdb users can call tvm::ffi::ReprPrint directly. - python/tvm/ir/attrs.py: add a Python-level __dict__ property on DictAttrs so that tvm_ffi's _add_class_attrs does not attempt to install a class descriptor named "__dict__" (Python forbids adding such a descriptor on extension-type subclasses via setattr). This fixes the import regression introduced by the bump to tvm_ffi 0.1.12.dev (which changed the field-registration guard from "not hasattr(cls, name)" to "name not in cls.__dict__").
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the tvm-ffi subproject, removes the legacy tvm/ir/repr.h header and its associated tvm::Dump helper functions, and replaces them with tvm::ffi::ReprPrint. It also updates reflection registrations across various TVM IR nodes to use SEqHashDefRecursive instead of SEqHashDef, and adds a __dict__ property to DictAttrs in Python to avoid conflicts with C++ reflection registration. There are no review comments, and I have no additional feedback to provide.
spectrometerHBH
approved these changes
May 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two-commit PR:
Bump
3rdparty/tvm-ffifrom3c35034to98d0029and migrate all 21 in-treeSEqHashDef()call sites toSEqHashDefRecursive()(the conservative variant matching the prior default behavior). Six let-style sites carryTODO(tqchen)comments indicating they should flip toSEqHashDefNonRecursiveafter the new tvm-ffi ships on pypi.Phase out
include/tvm/ir/repr.h. The bumped tvm-ffi now provides ostreamoperator<<forAny/ObjectRef/Variant/Optionaldirectly intvm/ffi/extra/dataclass.h, making the in-tree thin wrapper redundant. Rewrite 8 includers, renamesrc/ir/repr.cc→src/ir/access_path_repr.cc(preservesnode.AsRepr+ AccessPath/AccessStep__ffi_repr__registrations; drops zero-callertvm::Dump()), delete the header. Also fixes a Python-level import regression inpython/tvm/ir/attrs.pycaused by the bump: tvm_ffi 0.1.12.dev changes the field-registration guard fromnot hasattr(cls, name)toname not in cls.__dict__, which breaksDictAttrsbecauseDictAttrsNoderegisters a reflection field named"__dict__"— Python forbids installing a class descriptor with that name viasetattr. Fix: define__dict__as an explicit Python property onDictAttrsso the auto-installation is skipped.TODO follow-ups
After the new tvm-ffi releases on pypi, flip the 6
SEqHashDefRecursive()sites that carryTODO(tqchen)comments toSEqHashDefNonRecursive(). Locations are enumerated in the commit body of commit 1.Test plan
import tvm; tvm.cuda(0).existreturns True.tests/python/all-platform-minimal-test: 37 passed, 105 skipped.tests/python/relax/test_struct_info.py: 9 passed.git grep -nE 'SEqHashDef\(|"tvm/ir/repr\.h"'is empty.pre-commit run --all-filesclean.