[GitHub] [incubator-tvm] junrushao1994 commented on a change in pull request #5960: [Target] Migrate data structure of TargetNode

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

junrushao1994 commented on a change in pull request #5960:
URL: https://github.com/apache/incubator-tvm/pull/5960#discussion_r448642829



##########
File path: python/tvm/relay/qnn/op/legalizations.py
##########
@@ -229,18 +229,17 @@ def _shift(data, zero_point, out_dtype):
 def is_fast_int8_on_intel():
     """ Checks whether the hardware has support for fast Int8 arithmetic operations. """
     target = tvm.target.Target.current(allow_none=False)
-    intel_supported_arches = {'-mcpu=skylake-avx512', '-mcpu=cascadelake'}
-    return intel_supported_arches.intersection(set(target.options))
+    return target.mcpu in {'skylake-avx512', 'cascadelake'}
 
 def is_fast_int8_on_arm():
     """ Checks whether the hardware has support for fast Int8 arithmetic operations. """
     target = tvm.target.Target.current(allow_none=False)
-    return '+v8.2a,+dotprod' in ' '.join(target.options)
+    return '+v8.2a,+dotprod' in target.mattr

Review comment:
       Shall we make `mattr` be `Array<String>` instead?

##########
File path: src/target/codegen.cc
##########
@@ -47,7 +47,12 @@ runtime::Module Build(IRModule mod, const Target& target) {
           .value()) {
     mod = tir::transform::SkipAssert()(mod);
   }
-  std::string build_f_name = "target.build." + target->target_name;
+  std::string build_f_name;
+  if (target->id->name == "micro_dev") {

Review comment:
       This is a bit ad-hoc here. Later maybe we should register codegen entry function as an attribute attached to target_id

##########
File path: tests/micro/test_runtime_micro_on_arm.py
##########
@@ -33,7 +33,7 @@
 # Ex : export CMSIS_ST_PATH="/home/yourid/st/STM32Cube_FW_F7_V1.16.0/Drivers/CMSIS"
 DEV_CONFIG_A = micro.device.arm.stm32f746xx.generate_config("127.0.0.1", 6666)
 DEV_CONFIG_B = micro.device.arm.stm32f746xx.generate_config("127.0.0.1", 6666)
-TARGET = 'c -device=micro_dev'
+TARGET = 'micro_dev'

Review comment:
       breaking change: now `micro_dev` is a new target, instead of the `device_name` of target `c`

##########
File path: src/target/source/codegen_aocl.cc
##########
@@ -62,8 +62,9 @@ runtime::Module BuildAOCL(IRModule mod, std::string target_str, bool emulation)
   // AOCL supports fp64.
   cmd += " -Dcl_khr_fp64";
   Target target = Target::Create(target_str);
-  if (target->device_name != "") {
-    cmd += " -board=" + target->device_name;
+  Optional<String> device = target->GetAttr<String>("device");
+  if (device.defined()) {
+    cmd += " -board=" + device.value();

Review comment:
       Should we add `board` as aocl's attr?

##########
File path: python/tvm/target/target.py
##########
@@ -102,6 +78,40 @@ def current(allow_none=True):
         """
         return _ffi_api.GetCurrentTarget(allow_none)
 
+    @property
+    def max_num_threads(self):
+        return int(self.attrs["max_num_threads"])
+
+    @property
+    def thread_warp_size(self):
+        return int(self.attrs["thread_warp_size"])
+
+    @property
+    def device_name(self):
+        return str(self.attrs.get("device", ""))
+
+    @property
+    def model(self):
+        """Returns model from the target if it exists."""
+        return str(self.attrs.get("model", "unknown"))
+
+    @property
+    def mcpu(self):
+        """Returns the mcpu from the target if it exists."""
+        return str(self.attrs.get("mcpu", ""))
+
+    @property
+    def mattr(self):
+        """Returns the mattr from the target if it exists."""
+        return self.attrs.get("mattr", "")
+
+    @property
+    def libs(self):
+        if not self._libs:
+            self._libs = list(self.attrs.get("libs", ""))

Review comment:
       should we make it a dict instead?

##########
File path: python/tvm/target/target.py
##########
@@ -41,45 +47,15 @@ def __new__(cls):
         # Always override new to enable class
         obj = Object.__new__(cls)
         obj._keys = None
-        obj._options = None
         obj._libs = None
         return obj
 
     @property
     def keys(self):
         if not self._keys:
-            self._keys = [str(k) for k in self.keys_array]
+            self._keys = [str(k) for k in self.keys_]

Review comment:
       I am not super sure why we need this function...

##########
File path: src/target/target.cc
##########
@@ -35,6 +36,41 @@ using runtime::PackedFunc;
 using runtime::TVMArgs;
 using runtime::TVMRetValue;
 
+Target Target::CreateTarget(const std::string& name, const std::vector<std::string>& options) {
+  TargetId id = TargetId::Get(name);
+  ObjectPtr<TargetNode> target = make_object<TargetNode>();
+  target->id = id;
+  // tag is always empty
+  target->tag = "";
+  // parse attrs
+  target->attrs = id->ParseAttrsFromRawString(options);
+  String device_name = target->GetAttr<String>("device", "").value();
+  // create string representation
+  {
+    std::ostringstream str_repr;
+    str_repr << name;
+    for (const auto& s : options) {
+      str_repr << ' ' << s;
+    }
+    target->str_repr_ = str_repr.str();
+  }

Review comment:
       TODO: remove this




----------------------------------------------------------------
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:
users@infra.apache.org