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.git
The following commit(s) were added to refs/heads/main by this push:
new c0a305dfeb [TARGET] Fix round-trip reconstruction of targets with
canonicalizer-generated `feature.*` attrs (#18883)
c0a305dfeb is described below
commit c0a305dfeb0848ccd06763753eb11ace8d72e7e6
Author: Masahiro Hiramori <[email protected]>
AuthorDate: Sat Mar 7 09:39:34 2026 +0900
[TARGET] Fix round-trip reconstruction of targets with
canonicalizer-generated `feature.*` attrs (#18883)
Fix #18882
`TargetNode::ToConfig()` exports all target attrs, including derived
`feature.*` fields set by target canonicalizers. However,
`TargetInternal::FromConfig()` rejects these keys during schema
validation because they are not declared in the target kind schema. This
breaks round-tripping exported configs through `Target(config)`.
This PR strips `feature.*` keys from the config before
`ConfigSchema::Resolve`, then merges them back afterward. Canonicalizer
output is authoritative — if the canonicalizer re-emits a `feature.*`
key, it overwrites the preserved value. Unknown non-`feature.*` keys
continue to fail validation as before.
Changes:
- src/target/target.cc: Extract and re-merge `feature.*` keys around
schema resolution in `FromConfig()`
- tests/cpp/target_test.cc: Add tests for single-target round-trip,
nested-host round-trip, and continued rejection of unknown non-feature
keys
---------
Co-authored-by: gemini-code-assist[bot]
<176961590+gemini-code-assist[bot]@users.noreply.github.com>
---
src/target/target.cc | 35 +++++++++++++++++++++++++----
tests/cpp/target_test.cc | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/src/target/target.cc b/src/target/target.cc
index 91a5854b39..7d03d5e14c 100644
--- a/src/target/target.cc
+++ b/src/target/target.cc
@@ -330,11 +330,38 @@ ObjectPtr<TargetNode>
TargetInternal::FromConfig(ffi::Map<ffi::String, ffi::Any>
target->host = std::nullopt;
}
- // Step 3: Use ConfigSchema to validate types, apply defaults, and run
canonicalizer
+ // Step 3: Extract any "feature.*" keys from the input config before schema
validation.
+ // These are canonicalizer-owned metadata that may appear in exported
configs (via ToConfig())
+ // but are not part of the target kind's declared schema. We preserve them
across round-trip
+ // and let the canonicalizer's output take priority if it re-emits the same
key.
+ std::unordered_map<std::string, ffi::Any> saved_features;
+ {
+ std::vector<ffi::String> feature_keys;
+ for (const auto& kv : config) {
+ std::string key_str(kv.first);
+ if (key_str.size() > 8 && key_str.compare(0, 8, "feature.") == 0) {
+ saved_features[key_str] = kv.second;
+ feature_keys.push_back(kv.first);
+ }
+ }
+ for (const auto& k : feature_keys) {
+ config.erase(k);
+ }
+ }
+
+ // Step 4: Use ConfigSchema to validate types, apply defaults, and run
canonicalizer
// Note: structural keys (kind, tag, keys, device) pass through to
canonicalizer
ffi::Map<ffi::String, ffi::Any> resolved =
target->kind->schema_.Resolve(config);
- // Step 4: Extract structural fields from resolved config
+ // Step 5: Merge back preserved feature.* keys. Canonicalizer output is
authoritative:
+ // only restore a saved feature if the canonicalizer did not re-emit it.
+ for (const auto& kv : saved_features) {
+ if (resolved.find(ffi::String(kv.first)) == resolved.end()) {
+ resolved.Set(ffi::String(kv.first), kv.second);
+ }
+ }
+
+ // Step 6: Extract structural fields from resolved config
if (resolved.count(kTag)) {
if (auto tag = resolved[kTag].try_cast<ffi::String>()) {
target->tag = tag.value();
@@ -367,14 +394,14 @@ ObjectPtr<TargetNode>
TargetInternal::FromConfig(ffi::Map<ffi::String, ffi::Any>
resolved.erase(kKeys);
}
- // Step 5: Build attrs from resolved entries (excluding structural keys)
+ // Step 7: Build attrs from resolved entries (excluding structural keys)
resolved.erase(kKind);
std::unordered_map<ffi::String, ffi::Any> attrs;
for (const auto& kv : resolved) {
attrs[kv.first] = kv.second;
}
- // Step 6: If requested, query attributes from the device. User-specified
+ // Step 8: If requested, query attributes from the device. User-specified
// parameters take precedence over queried parameters.
int64_t from_device_id = -1;
if (auto it = attrs.find(kFromDevice); it != attrs.end()) {
diff --git a/tests/cpp/target_test.cc b/tests/cpp/target_test.cc
index df592f7608..342870e88f 100644
--- a/tests/cpp/target_test.cc
+++ b/tests/cpp/target_test.cc
@@ -222,6 +222,63 @@ TEST(TargetCreation, TargetParserProcessing) {
ASSERT_EQ(test_target->GetAttr<ffi::String>("mattr").value(), "cake");
}
+TEST(TargetCreation, RoundTripCanonicalizerFeatures) {
+ // Construct a target whose canonicalizer sets feature.test and transforms
mcpu
+ Target original(ffi::Map<ffi::String, ffi::Any>{
+ {"kind", ffi::String("TestTargetParser")},
+ {"mcpu", ffi::String("woof")},
+ });
+ ASSERT_EQ(original->GetAttr<ffi::String>("mcpu").value(), "super_woof");
+ ASSERT_EQ(original->GetAttr<bool>("feature.test").value(), true);
+
+ // Export to config and reconstruct
+ ffi::Map<ffi::String, ffi::Any> exported = original->ToConfig();
+ Target reconstructed(exported);
+
+ // Canonicalized attrs must survive the round-trip
+ // Note: mcpu gets canonicalized again (super_super_woof) because the
canonicalizer runs
+ ASSERT_TRUE(reconstructed->GetAttr<ffi::String>("mcpu").has_value());
+ ASSERT_EQ(reconstructed->GetAttr<bool>("feature.test").value(), true);
+ ASSERT_EQ(reconstructed->keys.size(), 1);
+ ASSERT_EQ(reconstructed->keys[0], "super");
+}
+
+TEST(TargetCreation, RoundTripCanonicalizerFeaturesNestedHost) {
+ // Construct a host target whose canonicalizer sets feature.test
+ Target host(ffi::Map<ffi::String, ffi::Any>{
+ {"kind", ffi::String("TestTargetParser")},
+ {"mcpu", ffi::String("woof")},
+ });
+ ASSERT_EQ(host->GetAttr<bool>("feature.test").value(), true);
+
+ // Attach it as host to another target
+ Target outer(ffi::Map<ffi::String, ffi::Any>{
+ {"kind", ffi::String("TestTargetKind")},
+ {"my_bool", true},
+ });
+ Target combined(outer, host);
+
+ // Export the outer target (includes nested host) and reconstruct
+ ffi::Map<ffi::String, ffi::Any> exported = combined->ToConfig();
+ Target reconstructed(exported);
+
+ // The nested host must reconstruct successfully with feature.* preserved
+ ffi::Optional<Target> reconstructed_host = reconstructed->GetHost();
+ ASSERT_TRUE(reconstructed_host.defined());
+ ASSERT_EQ(reconstructed_host.value()->GetAttr<bool>("feature.test").value(),
true);
+
ASSERT_TRUE(reconstructed_host.value()->GetAttr<ffi::String>("mcpu").has_value());
+}
+
+TEST(TargetCreationFail, UnknownNonFeatureKeyStillFails) {
+ // Verify that unknown non-feature.* keys still fail schema validation
+ ffi::Map<ffi::String, ffi::Any> config = {
+ {"kind", ffi::String("TestTargetParser")},
+ {"mcpu", ffi::String("woof")},
+ {"unknown_key", ffi::String("bad")},
+ };
+ ASSERT_THROW({ Target{config}; }, tvm::Error);
+}
+
TVM_REGISTER_TARGET_KIND("TestStringKind", kDLCPU)
.add_attr_option<ffi::String>("single")
.add_attr_option<ffi::Array<ffi::String>>("array")
