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



##########
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:
       I've added groupings to the example, I think you're right if they're all 
just blasted onto the screen it's a bit scary but in groups it's similar to 
other CLIs where you just scroll to the section you're interested in. We can 
also iterate on help as we go if we think it's getting overwhelming.
   
   Re: user vs developer, I've always assumed `tvmc` is a user facing tool so I 
started heading in that direction, @leandron is probably the right person to 
make the call though. 




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to