[GitHub] [tvm-rfcs] comaniac commented on a change in pull request #28: Command Line Composition from Internal Registry

[GitBox <gi...@apache.org>]

comaniac commented on a change in pull request #28:
URL: https://github.com/apache/tvm-rfcs/pull/28#discussion_r710315602



##########
File path: rfcs/0028-command-line-registry-composition.md
##########
@@ -0,0 +1,108 @@
+- Feature Name: Command Line Composition from Internal Registry
+- Start Date: 2021-08-24
+- RFC PR: [apache/tvm-rfcs#28](https://github.com/apache/tvm-rfcs/pull/28)
+- GitHub Issue: [apache/tvm#0000](https://github.com/apache/tvm/issues/0000)
+
+# Summary
+[summary]: #summary
+
+Introducing a standardised form for `tvmc` arguments to be populated from internal registries in TVM.
+
+# Motivation
+[motivation]: #motivation
+
+Currently, when a user uses `tvmc`, they present a target string:
+```
+tvmc --target="woofles -mcpu=woof, c -mattr=+mwoof"
+```
+
+Using a target string here means that the entire `--target` argument is used as an opaque pass-through to the internal `Target` string parser. It'd be great for a user to be able to compose these options and get meaningful help when building them up in `tvmc`.
+
+# Guide-level explanation
+[guide-level-explanation]: #guide-level-explanation
+
+Alongside support for a target string, `tvmc` would populate a series of other arguments specific to targets which match the attributes found in the TargetKindRegistry in `target_kind.cc`. For example, consider a subset of the `c` `Target`:
+
+```
+TVM_REGISTER_TARGET_KIND("c", kDLCPU)
+    .add_attr_option<String>("mcpu")
+```
+
+This would be translated at the `tvmc` level to:
+```bash
+tvmc --target=c \
+    --target-c-mcpu=cortex-m3
+```
+
+Which would then allow the user to compose together these options, such as:
+
+```bash
+tvmc --target=cmsisnn,c \ # Specifying multiple targets to enable in priority order
+    --target-cmsisnn-mattr=+dsp \
+    --target-c-mcpu=cortex-m3
+```
+
+# Reference-level explanation
+[reference-level-explanation]: #reference-level-explanation
+
+There already exists a mechanism which provides Python with `PassConfig` information, via:
+
+```c++
+// Function Registry
+TVM_REGISTER_GLOBAL("transform.ListConfigs").set_body_typed(PassContext::ListConfigs);
+// Actual call
+Map<String, Map<String, String>> PassContext::ListConfigs() {
+  return PassConfigManager::Global()->ListConfigs();
+}
+// Implementation
+Map<String, Map<String, String>> ListConfigs() {
+    Map<String, Map<String, String>> configs;
+    for (const auto& kv : key2vtype_) {
+        Map<String, String> metadata;
+        metadata.Set("type", kv.second.type_key);
+        configs.Set(kv.first, metadata);
+    }
+    return configs;
+}
+```
+
+This can be replicated to provide the same information for `TargetKind` or any other registry and provide this information to generate arguments in `tvmc` using `argparse`:
+```
+parser.add_argument(f"--target-{kind}-{attr}", ...)

Review comment:
       So the TVMC argument becomes like the following?
   ```
   tvmc --target=xx,c
   --target-xx-attr
   --target-cpu-mcpu
   --target-cpu-device
   --target-cpu-mtriple
   --target-cpu-mattr
   --target-cpu-mfloat-abi
   ```
   
   Seems a bit tedious. How about the following:
   ```
   tvmc --target=xx,c
   --target-xx "-attr=..."
   --target-cpu "-mcpu=x, -device=x, -mtriple=x, -mattr=x, -mfloat-abi=x"
   ```
   
   We can still leverage the same idea as list configs to show the detail attributes of each target in this approach. You just need to embed the extracted information to the argparse description.

##########
File path: rfcs/0028-command-line-registry-composition.md
##########
@@ -0,0 +1,108 @@
+- Feature Name: Command Line Composition from Internal Registry
+- Start Date: 2021-08-24
+- RFC PR: [apache/tvm-rfcs#28](https://github.com/apache/tvm-rfcs/pull/28)
+- GitHub Issue: [apache/tvm#0000](https://github.com/apache/tvm/issues/0000)
+
+# Summary
+[summary]: #summary
+
+Introducing a standardised form for `tvmc` arguments to be populated from internal registries in TVM.
+
+# Motivation
+[motivation]: #motivation
+
+Currently, when a user uses `tvmc`, they present a target string:
+```
+tvmc --target="woofles -mcpu=woof, c -mattr=+mwoof"
+```
+
+Using a target string here means that the entire `--target` argument is used as an opaque pass-through to the internal `Target` string parser. It'd be great for a user to be able to compose these options and get meaningful help when building them up in `tvmc`.

Review comment:
       I don't really understand this statement. In particular `compose these options and get meaningful help when building them up in tvmc`. Could you provide an example in this section, including the expected result (i.e., TVMC command and underlying behaviors), to further illustrate the motivation?




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org