[GitHub] [tvm-rfcs] areusch commented on a diff in pull request #78: Add Target Features RFC

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

areusch commented on code in PR #78:
URL: https://github.com/apache/tvm-rfcs/pull/78#discussion_r903117570


##########
rfcs/0078-target-features.md:
##########
@@ -0,0 +1,178 @@
+- Feature Name: target-features
+- Start Date: 2022-04-04
+- RFC PR: [apache/tvm-rfcs#78](https://github.com/apache/tvm-rfcs/pull/78)
+- GitHub Issue: [apache/tvm#0000](https://github.com/apache/tvm/issues/0000)
+
+# Summary
+[summary]: #summary
+Provide a standard and easily testable way to inspect features of a given target and provide them to the various parts of TVM which utilise that information.
+
+# Motivation
+[motivation]: #motivation
+TVM has multiple ways to define a `Target`s architectural features for use in deciding on schedules or other calculations, here's a few different ways we do this:
+
+* CPU to Feature Mapping: https://github.com/apache/tvm/blob/d2db9cb0d839e32778f461b77e59f6418282a511/python/tvm/target/arm_isa.py#L22-L39
+* Inspecting `Target` in utility functions: https://github.com/apache/tvm/blob/d2db9cb0d839e32778f461b77e59f6418282a511/python/tvm/topi/arm_cpu/arm_utils.py#L24-L70
+* Inspecting `Target` in utility functions inside legalization code: https://github.com/apache/tvm/blob/02fbaf0ed9120a8f95155e63de42459f230584aa/python/tvm/relay/qnn/op/legalizations.py#L350-L359
+* Inspecting `Target` inside the definition a strategy: https://github.com/apache/tvm/blob/b542724873140bb051492530d97a78b9b7b7983d/python/tvm/relay/op/strategy/arm_cpu.py#L232
+* Processing bespoke Compiler arguments: https://github.com/apache/tvm/blob/d2db9cb0d839e32778f461b77e59f6418282a511/src/relay/backend/contrib/cmsisnn/compiler_attrs.cc#L47-L70
+* Registered as a `PackedFunc` (https://github.com/apache/tvm/blob/24e5498021cecca2fe7d44149ce90efe28b6d930/python/tvm/topi/x86/utils.py#L21-L34) and then used as part of `Op` processing: https://github.com/apache/tvm/blob/24e5498021cecca2fe7d44149ce90efe28b6d930/src/relay/qnn/op/requantize_config.h#L58-L73
+
+This RFC aims to standardise the way in which we convert `Target` attributes into architectural features by processing them ahead of time.
+
+# Guide-level explanation
+[guide-level-explanation]: #guide-level-explanation
+
+An additional property `features` will be added to the `Target` which is created at the time of instantiation, this will be populated by inferred features of the `Target` such as architectural extensions or bus sizes. The main distinction is that `features` are inferred from the `Target` `attrs` rather than being passed in.
+
+An example of the new `features` attribute will be illustrated using examples targeting TVM for Arm(R) Cortex(R)-M4.
+
+The `Target` specifies the specific CPU in the `attrs` and uses that to create the `features` object representing the architectural extensions of the `Target`, which can then be accessed using the `GetFeature` method similar to `GetAttr`:
+
+```c++
+Target my_target("c -mcpu=cortex-m4");
+my_target->GetFeature<Bool>("is_aarch64", false); // false
+my_target->GetFeature<Bool>("has_dsp", false); // true
+```
+
+```python
+my_target = Target("c -mcpu=cortex-m4")
+my_target.features.is_aarch64 # false
+my_target.features.has_dsp # true
+```
+
+This means that instead of the current:
+
+```python
+isa = arm_isa.IsaAnalyzer(target)
+if isa.has_dsp_support:
+    do_dsp_stuff()
+```
+
+The `Target` can be directly inspected:
+
+```python
+if target.features.dsp:

Review Comment:
   should we adopt any namespacing? if so how would we represent that with this syntax?



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