You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/06/29 21:38:31 UTC

[GitHub] [incubator-tvm] junrushao1994 opened a new pull request #5960: [Target] Migrate data structure of TargetNode

junrushao1994 opened a new pull request #5960:
URL: https://github.com/apache/incubator-tvm/pull/5960


   Per RFC: [TVM Target Specification](https://discuss.tvm.ai/t/rfc-tvm-target-specification/6844?u=junrushao1994)
   
   


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



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

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



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

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #5960:
URL: https://github.com/apache/incubator-tvm/pull/5960#issuecomment-653105688


   @areusch Hey Andrew, this PR changes the target definition of micro-dev from `c -device=micro_dev` to `micro_dev`


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



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

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5960:
URL: https://github.com/apache/incubator-tvm/pull/5960#discussion_r448655057



##########
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:
       you can make the call




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



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

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #5960:
URL: https://github.com/apache/incubator-tvm/pull/5960#discussion_r449147659



##########
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:
       @areusch yeah, this PR makes it happen. The `micro_dev` is promoted as a new target name, with "codegen_c" as its code generator.




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



[GitHub] [incubator-tvm] tqchen commented on pull request #5960: [Target] Migrate data structure of TargetNode

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5960:
URL: https://github.com/apache/incubator-tvm/pull/5960#issuecomment-653182013


   Thanks @junrushao1994 @areusch 


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



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

Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #5960:
URL: https://github.com/apache/incubator-tvm/pull/5960#discussion_r449140041



##########
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:
       in the future i'd like to remove `micro_dev` as a special target name, but we haven't come to consensus on that, so it's fine for now. 

##########
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:
       I guess the other option would be to add a --runtime attr to c codegen, but I can also do that in a follow-up




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



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

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #5960:
URL: https://github.com/apache/incubator-tvm/pull/5960#discussion_r449148119



##########
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:
       Sure thing, let's discuss about it in the future :-)




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



[GitHub] [incubator-tvm] tqchen merged pull request #5960: [Target] Migrate data structure of TargetNode

Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #5960:
URL: https://github.com/apache/incubator-tvm/pull/5960


   


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