Skip to content

[REFACTOR][IR] Phase out src/ir/structural_{hash,equal}.cc to tvm-ffi#19613

Merged
tlopex merged 4 commits into
apache:mainfrom
tqchen:tvm-structural-hash-equal-to-ffi
May 26, 2026
Merged

[REFACTOR][IR] Phase out src/ir/structural_{hash,equal}.cc to tvm-ffi#19613
tlopex merged 4 commits into
apache:mainfrom
tqchen:tvm-structural-hash-equal-to-ffi

Conversation

@tqchen

@tqchen tqchen commented May 26, 2026

Copy link
Copy Markdown
Member

Summary

The tvm-ffi layer now provides fully featured structural-hash and structural-equal APIs (including GetFirstStructuralMismatch with AccessPath pair output). The two TUs src/ir/structural_hash.cc and src/ir/structural_equal.cc had become thin adapters with no logic of their own — they forwarded to tvm-ffi and registered the results as node.Structural* globals for Python to call. This PR removes the indirection.

  • Commit A ([REFACTOR][IR]): relocates the ffi::ModuleObj and ffi::TensorObj __data_to_json__/__data_from_json__ TypeAttrDef registrations from structural_hash.cc into src/ir/module.cc and src/runtime/tensor.cc respectively, both of which already have a TVM_FFI_STATIC_INIT_BLOCK for those types.
  • Commit B ([REFACTOR][PYTHON]): rewrites the four Python wrappers in tvm.ir.base (structural_equal, get_first_structural_mismatch, assert_structural_equal, structural_hash) to call tvm_ffi._ffi_api directly, bypassing the now-redundant node.Structural* globals. assert_structural_equal reconstructs the same diagnostic message in Python using TVMScriptPrinterScript with path_to_underline.
  • Commit C ([REFACTOR][IR]): deletes src/ir/structural_hash.cc and src/ir/structural_equal.cc whose remaining content (the node.Structural* FFI global registrations) is now unused.

tqchen added 3 commits May 26, 2026 15:14
…e homes

The serialization TypeAttrDef registrations for ffi::ModuleObj and
ffi::TensorObj were sitting in src/ir/structural_hash.cc as a
historical accident; their natural home is alongside the rest of
the type's machinery. This commit relocates them to src/ir/module.cc
and src/runtime/tensor.cc respectively, both of which already host a
TVM_FFI_STATIC_INIT_BLOCK for the same type.
…ckends

Python wrappers in tvm.ir.base were forwarding to TVM-side adapters
(node.StructuralEqual / .StructuralHash / .GetFirstStructuralMismatch)
which only existed to bridge into tvm-ffi. Cut out the middle layer:
call the tvm-ffi APIs directly. assert_structural_equal rebuilds the
diagnostic message in Python using the existing TVMScript printer
with path_to_underline, byte-identical to the C++ adapter's output.
With Module/Tensor serialization attrs relocated to their type homes
and Python callers migrated to tvm-ffi directly, these two TU's hold
only the now-unused node.Structural* FFI globals. Delete the files.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors structural equality and hashing mechanisms by migrating FFI registrations. It deletes src/ir/structural_equal.cc and src/ir/structural_hash.cc, moving JSON serialization/deserialization logic for ModuleObj and TensorObj directly to src/ir/module.cc and src/runtime/tensor.cc respectively. In Python, assert_structural_equal is updated to provide detailed mismatch paths using GetFirstStructuralMismatch and the script printer. A review comment suggests using a direct StructuralEqual API instead of GetFirstStructuralMismatch in structural_equal to avoid path-tracking overhead and improve performance.

Comment thread python/tvm/ir/base.py Outdated
…undant convert

Switches structural_equal / get_first_structural_mismatch /
assert_structural_equal / structural_hash from _ffi_api.X to the
higher-level tvm_ffi.X wrappers (smaller FFI api surface), and drops
the redundant tvm.runtime.convert() calls since tvm-ffi auto-converts.
@tlopex tlopex merged commit d37b6ab into apache:main May 26, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants