junrushao1994 commented on a change in pull request #6218:
URL: https://github.com/apache/incubator-tvm/pull/6218#discussion_r470822681
##########
File path: src/target/target.cc
##########
@@ -162,14 +314,164 @@ Target Target::Create(const String& target_str) {
return CreateTarget(splits[0], {splits.begin() + 1, splits.end()});
}
+ObjectRef TargetNode::ParseAttr(const ObjectRef& obj,
+ const TargetKindNode::ValueTypeInfo& info)
const {
+ if (info.type_index ==
Integer::ContainerType::_GetOrAllocRuntimeTypeIndex()) {
+ const auto* v = obj.as<IntImmNode>();
+ CHECK(v != nullptr) << "Expect type 'int', but get: " << obj->GetTypeKey();
+ return GetRef<Integer>(v);
+ }
+ if (info.type_index == String::ContainerType::_GetOrAllocRuntimeTypeIndex())
{
+ const auto* v = obj.as<StringObj>();
+ CHECK(v != nullptr) << "Expect type 'str', but get: " << obj->GetTypeKey();
+ return GetRef<String>(v);
+ }
+ if (info.type_index == Target::ContainerType::_GetOrAllocRuntimeTypeIndex())
{
+ CHECK(obj->IsInstance<MapNode>())
+ << "Expect type 'dict' to construct Target, but get: " <<
obj->GetTypeKey();
+ return Target::FromConfig(Downcast<Map<String, ObjectRef>>(obj));
+ }
+ if (info.type_index == ArrayNode::_GetOrAllocRuntimeTypeIndex()) {
+ CHECK(obj->IsInstance<ArrayNode>()) << "Expect type 'list', but get: " <<
obj->GetTypeKey();
+ Array<ObjectRef> array = Downcast<Array<ObjectRef>>(obj);
+ std::vector<ObjectRef> result;
+ int i = 0;
+ for (const ObjectRef& e : array) {
+ ++i;
+ try {
+ result.push_back(TargetNode::ParseAttr(e, *info.key));
+ } catch (const dmlc::Error& e) {
+ LOG(FATAL) << "Error occurred when parsing element " << i << " of the
array: " << array
+ << ". Details:\n"
+ << e.what();
+ }
+ }
+ return Array<ObjectRef>(result);
+ }
+ if (info.type_index == MapNode::_GetOrAllocRuntimeTypeIndex()) {
+ CHECK(obj->IsInstance<MapNode>()) << "Expect type 'dict', but get: " <<
obj->GetTypeKey();
+ std::unordered_map<ObjectRef, ObjectRef, ObjectHash, ObjectEqual> result;
+ for (const auto& kv : Downcast<Map<ObjectRef, ObjectRef>>(obj)) {
+ ObjectRef key, val;
+ try {
+ key = TargetNode::ParseAttr(kv.first, *info.key);
+ } catch (const tvm::Error& e) {
+ LOG(FATAL) << "Error occurred when parsing a key of the dict: " <<
kv.first
+ << ". Details:\n"
+ << e.what();
+ }
+ try {
+ val = TargetNode::ParseAttr(kv.second, *info.val);
+ } catch (const tvm::Error& e) {
+ LOG(FATAL) << "Error occurred when parsing a value of the dict: " <<
kv.second
+ << ". Details:\n"
+ << e.what();
+ }
+ result[key] = val;
+ }
+ return Map<ObjectRef, ObjectRef>(result);
+ }
+ LOG(FATAL) << "Unsupported type registered: \"" << info.type_key
+ << "\", and the type given is: " << obj->GetTypeKey();
+ throw;
+}
+
+Target Target::FromConfig(const Map<String, ObjectRef>& config_dict) {
+ const String kKind = "kind";
+ const String kTag = "tag";
+ const String kKeys = "keys";
+ std::unordered_map<std::string, ObjectRef> config(config_dict.begin(),
config_dict.end());
+ ObjectPtr<TargetNode> target = make_object<TargetNode>();
+ // parse 'kind'
+ if (config.count(kKind)) {
+ const auto* kind = config[kKind].as<StringObj>();
+ CHECK(kind != nullptr) << "AttributeError: Expect type of field 'kind' is
string, but get: "
+ << config[kKind]->GetTypeKey();
+ target->kind = TargetKind::Get(GetRef<String>(kind));
+ config.erase(kKind);
+ } else {
+ LOG(FATAL) << "AttributeError: Field 'kind' is not found";
+ }
+ // parse "tag"
+ if (config.count(kTag)) {
+ const auto* tag = config[kTag].as<StringObj>();
+ CHECK(tag != nullptr) << "AttributeError: Expect type of field 'tag' is
string, but get: "
+ << config[kTag]->GetTypeKey();
+ target->tag = GetRef<String>(tag);
+ config.erase(kTag);
+ } else {
+ target->tag = "";
+ }
+ // parse "keys"
+ if (config.count(kKeys)) {
+ std::vector<String> keys;
+ // user provided keys
+ const auto* cfg_keys = config[kKeys].as<ArrayNode>();
+ CHECK(cfg_keys != nullptr)
+ << "AttributeError: Expect type of field 'keys' is an Array, but get: "
+ << config[kTag]->GetTypeKey();
+ for (const ObjectRef& e : *cfg_keys) {
+ const auto* key = e.as<StringObj>();
+ CHECK(key != nullptr) << "AttributeError: Expect 'keys' to be an array
of strings, but it "
+ "contains an element of type: "
+ << e->GetTypeKey();
+ keys.push_back(GetRef<String>(key));
+ }
+ // add device name
+ if (config_dict.count("device") &&
config_dict.at("device")->IsInstance<StringObj>()) {
+ keys.push_back(Downcast<String>(config_dict.at("device")));
+ }
+ // add default keys
+ for (const auto& key : target->kind->default_keys) {
+ keys.push_back(key);
+ }
+ // de-duplicate keys
+ target->keys = DeduplicateKeys(keys);
Review comment:
**In response to the second example.** We can see that the second
example exposes an extreme case that the keys can be semantically unnatural,
i.e. typically there are should not be targets on both cpu and gpu. It is
typically impossible.
To keep the sanity of the workflow, a check must be conducted somewhere; or
we have to assume the users are all "grown adults" who can write correct target
(which IMO is still valid assumption). IIUC the problem is that should target
creation do such check? IMHO, however, we don't need it to be constant magic
strings fixed in C++ side of "target", for the following reasons:
* Imagine an external developer who installs TVM using pip/conda, and he
just wanted to write all operators in python using his own specific keys for
his specific arm device. Are we harsh enough to ask him to recompile the entire
TVM codebase, just to allow this key to exist? It doesn't really let you gain
anything, but hurts flexibility.
* It is hardly possible to enumerate all invalid cases. As those two
examples you proposed, they are "somewhat invalid" in different ways, reversed
key order, semantically incompatible keys, etc. Have they really covered all
the cases? IIUC, no, not really. I could imagine thousands of possible
combinations that are semantically wrong. For example, "mtriple", "march",
"mcpu" are incompatible. It is just impossible to enumerate all of them.
**Syntactical vs semantical.** We aim at solving syntactical problems in
this series of PRs, but as I have mentioned in this post, semantical issues are
never possible to be enumerated or solved, and shouldn't be solved under the
scope of target. What we really want is a high-level wrap of target creation
that makes semantic checking possible, or the "target tag" system that
guarantees correct outcome.
Thank you for the very constructive comments! Those comments really made me
rethink and understand more deeply into all those problems. Thank you Cody!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
