[REFACTOR][NODE] Remove node redirect headers#18829
[REFACTOR][NODE] Remove node redirect headers#18829tqchen wants to merge 9 commits intoapache:mainfrom
Conversation
This PR removes the pure redirect headers from include/tvm/node/ (node.h, reflection.h) and moves thin wrappers (serialization, structural_equal, structural_hash) to include/tvm/ir/. Downstream code should use ffi:: directly for core types. Main changes: - Delete include/tvm/node/node.h (pure redirect) — update 24 includers - Delete include/tvm/node/reflection.h (0 includers, unused declaration) - Move serialization.h/cc, structural_equal.h/cc, structural_hash.h/cc to include/tvm/ir/ and src/ir/ — leave forwarding headers at old paths - Extend include/tvm/runtime/object.h to export Object, ObjectPtrEqual, ObjectPtrHash to tvm:: namespace (previously via node.h) - Add explicit includes for node/cast.h, node/repr_printer.h, node/script_printer.h to ir/expr.h (previously transitive via node.h)
Summary of ChangesHello @tqchen, 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 significantly refactors the internal header organization by streamlining 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. Changelog
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. 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the include/tvm/node/ namespace by removing aggregation headers and moving several node-related utilities to include/tvm/ir/. The changes are extensive but appear to be consistently applied across the codebase. Forwarding headers are added for backward compatibility. I have reviewed the changes and found no issues.
- Phase out BaseValueEqual: inline all operator() methods directly into StructuralEqual, removing the unnecessary base class - Fix clang-format lint: correct include ordering in structural_equal.cc and structural_hash.cc
tqchen
changed the title
… directly - Replace tvm::StructuralEqual with using ffi::StructuralEqual - Replace tvm::StructuralHash (and BaseValueHash) with using ffi::StructuralHash - Make tvm/ir/serialization.h a thin forwarding header to ffi/extra/serialization.h - Remove tvm::StructuralEqual::operator() and tvm::StructuralHash::operator() implementations that were just forwarding to ffi - Fix one 3-arg StructuralEqual call to use ffi::StructuralEqual::Equal directly - Add missing transitive includes (cmath, runtime/tensor.h, device_api.h) that were previously pulled in through the removed headers
Tests need the fully-qualified namespace since they may not have `using namespace tvm;` in scope.
3 participants
This PR begins the cleanup of the
include/tvm/node/