[REFACTOR][FFI] Cleanup ffi indirections in tvm headers + switch logging.h to ffi/error.h where only ICHECK/THROW are used#19483
Merged
tlopex merged 4 commits intoApr 30, 2026
Conversation
Remove pure type re-export aliases (`using X = ffi::Y`) from tvm include/ headers and qualify all call sites to use the canonical ffi names directly: - `MemoryScope = ffi::String` in global_info.h and virtual_device.h - `Region = ffi::Array<Range>` in tirx/var.h (+ `using tirx::Region` pull-in from arith/bound.h) - `AccessPath = ffi::reflection::AccessPath` in script/printer/doc.h and ir_docsifier.h - `tvm_index_t = ffi::Shape::index_type` in runtime/data_type.h - `FCallPacked = ffi::String` in relax/op_attr_types.h - `TGlobalSymbol = ffi::String` and `TScriptPrinterName = ffi::String` in tirx/op_attr_types.h (also the TVM_TIR_REGISTER_OP macro key) - `Container = ffi::TensorObj` in runtime/tensor.h Remove free-function pull-ins via `using ffi::X`: - `using ffi::EnvErrorAlreadySet` from runtime/logging.h - `using ffi::GetDataSize/IsAligned/IsContiguous` from runtime/tensor.h - `using ffi::DLDataTypeToString/StringToDLDataType` from runtime/data_type.h - `using ffi::Any/Function/PackedArgs` from target/codegen.h Qualify all ~120 call sites across ~100 files to use the fully qualified `ffi::*` names. Function-signature aliases (FCompute, FReduce, meta_schedule Fxxx, NodeFunctor FType, etc.) are kept as they name callback signatures, not type re-exports. Also adds explicit `#include <tvm/runtime/logging.h>` to files that use LOG/VLOG/DLOG macros and also had alias rewrites, to ensure each commit is independently buildable once the transitive dependency on logging.h through ir/op.h is removed by the follow-on logging include cleanup commit.
… to tvm/ffi/error.h For 64 files (7 in include/, 57 in src/) that include <tvm/runtime/logging.h> but only use TVM_FFI_ICHECK / TVM_FFI_THROW macros (no LOG/VLOG/DLOG logging features), switch the include to <tvm/ffi/error.h> which provides those macros directly. Files that use LOG(WARNING), LOG(INFO), VLOG, DLOG, VLOG_CONTEXT, or similar logging features keep <tvm/runtime/logging.h>. No ICHECK → TVM_FFI_ICHECK rewrites needed: bare ICHECK is already absent across include/ and src/ (only TVM_FFI_ICHECK is used). Also adds explicit <tvm/runtime/logging.h> to ~70 src/ files that use LOG/VLOG/DLOG macros but previously relied on a transitive include chain through include/tvm/ir/op.h (which switched from logging.h to ffi/error.h). Making these includes explicit is correct hygiene regardless; switching op.h's include only makes it visible.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request performs a large-scale refactoring to improve code modularity and dependency management. Key changes include replacing various tvm/runtime/logging.h includes with tvm/ffi/error.h to reduce dependencies, replacing deprecated Region aliases with ffi::Array<Range>, and updating AccessPath references to ffi::reflection::AccessPath. Additionally, several TGlobalSymbol and FCallPacked attributes have been updated to use ffi::String. I have no feedback to provide as these changes appear to be a consistent, codebase-wide cleanup.
Commit 1 of this PR removed `using ffi::IsContiguous;`, `using ffi::DLDataTypeToString;`, and similar free-function pull-ins from public headers. The cpptest sources in `tests/cpp/` still referenced those names unqualified through `tvm::runtime::*`, which broke the cpu/wasm CI builds. Qualify all such references in `tests/cpp/` with the canonical `ffi::*` namespace. Files: tests/cpp/ndarray_test.cc, tests/cpp/tir_scalable_datatype.cc
After the previous commit deleted all ffi:: indirection aliases, review identified 9 aliases that carry distinct semantic meaning beyond a mere namespace shortcut and should be preserved: Region = ffi::Array<Range> (tirx::, arith::) MemoryScope = ffi::String (global_info.h, virtual_device.h) TGlobalSymbol = ffi::String (tirx/op_attr_types.h) TScriptPrinterName = ffi::String (tirx/op_attr_types.h) FCallPacked = ffi::String (relax/op_attr_types.h) AccessPath = ffi::reflection::AccessPath (script/printer namespace only) Re-add these aliases in the relevant headers and revert all call-site rewrites (template arguments, variable types, return types) to use the alias names again. String key literals (e.g., "FCallPacked") are left unchanged — they are registry lookup keys, not type names. Files outside the printer namespace (src/s_tir/analysis/is_pure_function.cc, src/ir/script_printer.cc) keep the fully-qualified ffi::reflection::AccessPath.
tlopex
approved these changes
Apr 30, 2026
MasterJH5574
added a commit
to MasterJH5574/tvm
that referenced
this pull request
May 11, 2026
Six CUDA sources in src/runtime/contrib used LOG(FATAL) via transitive includes that apache#19483 trimmed; add the explicit <tvm/runtime/logging.h> include to thrust.cu, attention_kernels.cu, and the four cutlass kernel headers (fp16/fp8 sm90/sm100, gemm_runner, fp8_groupwise_scaled_gemm). cache_kernels.cu used the bare Array{...} alias that apache#19483 removed; switch to ffi::Array<Tensor>{...}. attention_kernels.cu registered FFI functions whose parameters were raw DLTensor*; the new reflection registry requires TypeSchema, so wrap both TVM_FFI_STATIC_INIT_BLOCK registrations to take Tensor and forward to the unchanged launchers via GetDLTensorPtr() (with const_cast for the output tensors, matching the mt_random_engine / cudnn pattern).
MasterJH5574
added a commit
to MasterJH5574/tvm
that referenced
this pull request
May 11, 2026
Six CUDA sources in src/runtime/contrib used LOG(FATAL) via transitive includes that apache#19483 trimmed; add the explicit <tvm/runtime/logging.h> include to thrust.cu, attention_kernels.cu, and the four cutlass kernel headers (fp16/fp8 sm90/sm100, gemm_runner, fp8_groupwise_scaled_gemm). cache_kernels.cu used the bare Array{...} alias that apache#19483 removed; switch to ffi::Array<Tensor>{...}. attention_kernels.cu registered FFI functions whose parameters were raw DLTensor*; the new reflection registry requires TypeSchema, so wrap both TVM_FFI_STATIC_INIT_BLOCK registrations to take Tensor and forward to the unchanged launchers via GetDLTensorPtr() (with const_cast for the output tensors, matching the mt_random_engine / cudnn pattern).
tlopex
pushed a commit
that referenced
this pull request
May 12, 2026
Six CUDA sources in src/runtime/contrib used LOG(FATAL) via transitive includes that #19483 trimmed; add the explicit <tvm/runtime/logging.h> include to thrust.cu, attention_kernels.cu, and the four cutlass kernel headers (fp16/fp8 sm90/sm100, gemm_runner, fp8_groupwise_scaled_gemm). cache_kernels.cu used the bare Array{...} alias that #19483 removed; switch to ffi::Array<Tensor>{...}. attention_kernels.cu registered FFI functions whose parameters were raw DLTensor*; the new reflection registry requires TypeSchema, so wrap both TVM_FFI_STATIC_INIT_BLOCK registrations to take Tensor and forward to the unchanged launchers via GetDLTensorPtr() (with const_cast for the output tensors, matching the mt_random_engine / cudnn pattern).
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
2 participants
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.
Background
This PR continues the FFI migration work. It focuses on two cleanup themes:
Theme 1: Drop ffi indirection aliases from TVM headers
Many TVM headers contained
usingre-export aliases like:These aliases were introduced as a transitional shim. Now that the codebase has stabilized,
they create confusion about which namespace owns each type. This commit removes these aliases
and rewrites all call sites to use
ffi::names directly.Files changed: ~103 files across
include/andsrc/.Theme 2: Switch icheck-only callers from
runtime/logging.htoffi/error.h64 files were
#include <tvm/runtime/logging.h>solely to useTVM_FFI_ICHECKand/orTVM_FFI_THROW. Those macros are now directly declared in<tvm/ffi/error.h>, so theheavier logging header is not needed for that purpose.
These files now include
<tvm/ffi/error.h>instead, keeping the dependency chain leaner.Files that also use
LOG(...)/VLOG(...)/DLOG(...)logging macros retain theruntime/logging.hinclude unchanged.Files changed: ~120 files across
include/andsrc/.Testing
tests/python/all-platform-minimal-test/: 16 passed, 126 skippedtests/python/tirx-base/: 251 passed, 23 skipped