This is an automated email from the ASF dual-hosted git repository.
tqchen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm-ffi.git
The following commit(s) were added to refs/heads/main by this push:
new 0292ae84 [REFACTOR][EXTRA] Move structural-mutate error construction
out of the hot path (#744)
0292ae84 is described below
commit 0292ae8419c630ffdf2a6a40492598388eb27f13
Author: Tianqi Chen <[email protected]>
AuthorDate: Fri Sep 4 22:03:02 2026 -0400
[REFACTOR][EXTRA] Move structural-mutate error construction out of the hot
path (#744)
`TryLink` inlines into the per-node dispatch function, so two error
constructions that only the matched path can reach put their string and
`Error` construction into a stack frame every node pays for.
This follows `BadStructuralMutateHookError`, which is already
`TVM_FFI_COLD_CODE static` for exactly this reason — its doc comment
says *"Inlined, its three string literals and Error construction land in
the traversal's hot path for no reason."* These were the two sites it
missed.
No semantic change: both checks still fire identically, only the
location of their error construction moves.
Per-node dispatch function: stack frame 216 → 120 bytes, 358 → 318
instructions. Both helpers land in `.text.unlikely.*`.
---
include/tvm/ffi/extra/structural_mutate.h | 38 +++++++++++++++++++++----------
src/ffi/extra/structural_mutate.cc | 3 +--
2 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/include/tvm/ffi/extra/structural_mutate.h
b/include/tvm/ffi/extra/structural_mutate.h
index f2164c09..b782b161 100644
--- a/include/tvm/ffi/extra/structural_mutate.h
+++ b/include/tvm/ffi/extra/structural_mutate.h
@@ -612,17 +612,24 @@ namespace details {
/// \endcond
+// Out of line so its strings and Error construction stay out of the hot path
of whatever hook
+// body TVM_FFI_S_MUTATE_ASSIGN_OR_RETURN expands into. Same reason as
+// BadStructuralMutateHookError.
+TVM_FFI_COLD_CODE inline TVMFFIAny SMutateDeclaredTypeErrorRaw() noexcept {
+ return AnyUnsafe::MoveAnyToTVMFFIAny(
+ Any(Error("TypeError", "structural mutate result does not match the
declared type", "")));
+}
+
/// \cond Doxygen_Suppress
-#define TVM_FFI_S_MUTATE_ASSIGN_OR_RETURN_IMPL_(Result, Type, Name,
ResultExpr) \
- auto Result = (ResultExpr); /* NOLINT(bugprone-macro-parentheses) */
\
- TVM_FFI_S_MUTATE_MAYBE_EARLY_RETURN(Result);
\
- if
(TVM_FFI_PREDICT_FALSE(!::tvm::ffi::details::AnyUnsafe::CheckAnyStrict<Type>(
\
- ::tvm::ffi::details::ExpectedUnsafe::GetData(Result)))) {
\
- return
::tvm::ffi::details::AnyUnsafe::MoveAnyToTVMFFIAny(::tvm::ffi::Any(::tvm::ffi::Error(
\
- "TypeError", "structural mutate result does not match the declared
type", ""))); \
- }
\
- Type Name = /* NOLINT(bugprone-macro-parentheses) */
\
- ::tvm::ffi::details::AnyUnsafe::MoveFromAnyAfterCheck<Type>(
\
+#define TVM_FFI_S_MUTATE_ASSIGN_OR_RETURN_IMPL_(Result, Type, Name,
ResultExpr) \
+ auto Result = (ResultExpr); /* NOLINT(bugprone-macro-parentheses) */
\
+ TVM_FFI_S_MUTATE_MAYBE_EARLY_RETURN(Result);
\
+ if
(TVM_FFI_PREDICT_FALSE(!::tvm::ffi::details::AnyUnsafe::CheckAnyStrict<Type>( \
+ ::tvm::ffi::details::ExpectedUnsafe::GetData(Result)))) {
\
+ return ::tvm::ffi::details::SMutateDeclaredTypeErrorRaw();
\
+ }
\
+ Type Name = /* NOLINT(bugprone-macro-parentheses) */
\
+ ::tvm::ffi::details::AnyUnsafe::MoveFromAnyAfterCheck<Type>(
\
::std::move(::tvm::ffi::details::ExpectedUnsafe::GetData(Result)))
/// \endcond
@@ -710,6 +717,14 @@ class StructuralMapMutatorBaseObj : public
StructuralMutatorObj {
: StructuralMutatorObj(vtable) {}
protected:
+ /// \cond Doxygen_Suppress
+ // Out of line so its strings stay out of the per-node dispatch function,
which TryLink inlines
+ // into. Shared by both map mutators: the typed one here and the dynamic one
in the .cc.
+ TVM_FFI_COLD_CODE static Expected<Any> SMutateDescentTypeError() noexcept {
+ return Unexpected(Error("TypeError", "structural mutate: descent changed
the node type", ""));
+ }
+ /// \endcond
+
/*!
* \brief Dispatch variable-remap lookup through the mutator vtable.
* \param mutator The erased callback-aware mutator.
@@ -1000,8 +1015,7 @@ class StructuralMapMutatorObj : public
StructuralMapMutatorBaseObj {
// required to preserve the type, so failing here means some hook
broke that.
std::optional<TSub> descended_sub = mapped_value.template as<TSub>();
if (TVM_FFI_PREDICT_FALSE(!descended_sub.has_value())) {
- return Unexpected(
- Error("TypeError", "structural mutate: descent changed the
node type", ""));
+ return SMutateDescentTypeError();
}
return InvokeCallbackLink(callback, *std::move(descended_sub), kind);
}
diff --git a/src/ffi/extra/structural_mutate.cc
b/src/ffi/extra/structural_mutate.cc
index 1705046c..4627d3a7 100644
--- a/src/ffi/extra/structural_mutate.cc
+++ b/src/ffi/extra/structural_mutate.cc
@@ -214,8 +214,7 @@ class StructuralMapDynMutatorObj : public
StructuralMapMutatorBaseObj {
// registered type index is the same target, so recheck against it.
if (TVM_FFI_PREDICT_FALSE(
!RuntimeTypeIndexMatch(mapped_value.type_index(),
link_type_index))) {
- *out =
- Unexpected(Error("TypeError", "structural mutate: descent changed
the node type", ""));
+ *out = SMutateDescentTypeError();
UpdateVisitErrorContext(*out, mapped_value);
return true;
}