leandron commented on a change in pull request #7462:
URL: https://github.com/apache/tvm/pull/7462#discussion_r578324044



##########
File path: src/target/target.cc
##########
@@ -373,6 +373,15 @@ Target::Target(const Map<String, ObjectRef>& config) {
   data_ = std::move(target);
 }
 
+Target::Target(Target target, Target host) {
+  ObjectPtr<TargetNode> n = make_object<TargetNode>(*target.get());
+  CHECK(!n->host.defined())
+      << "ValueError: Adding a host to a target whose host field has been 
defined";

Review comment:
       Please correct me if my question doesn't make sense, but am I correct to 
assume that this check will always pass, as you're adding the field directly to 
the macro ` TVM_REGISTER_TARGET_KIND`, which is used by all existing targets, 
and therefore `host` is a field for all of them?

##########
File path: tests/python/unittest/test_target_target.py
##########
@@ -158,6 +155,61 @@ def test_list_kinds():
     assert all(isinstance(target_name, str) for target_name in targets)
 
 
+def test_target_host_tags():
+    tgt = tvm.target.Target("nvidia/jetson-nano", "nvidia/geforce-rtx-2080-ti")
+    assert tgt.kind.name == "cuda"
+    assert tgt.attrs["arch"] == "sm_53"
+    assert tgt.attrs["shared_memory_per_block"] == 49152
+    assert tgt.attrs["max_threads_per_block"] == 1024
+    assert tgt.attrs["thread_warp_size"] == 32
+    assert tgt.attrs["registers_per_block"] == 32768
+    assert tgt.host.kind.name == "cuda"
+    assert tgt.host.attrs["arch"] == "sm_75"
+    assert tgt.host.attrs["shared_memory_per_block"] == 49152
+    assert tgt.host.attrs["max_threads_per_block"] == 1024
+    assert tgt.host.attrs["thread_warp_size"] == 32
+    assert tgt.host.attrs["registers_per_block"] == 65536
+
+
+def test_target_host_tag_dict():
+    tgt = tvm.target.Target("nvidia/jetson-nano", {"kind": "llvm"})
+    assert tgt.kind.name == "cuda"
+    assert tgt.attrs["arch"] == "sm_53"
+    assert tgt.attrs["shared_memory_per_block"] == 49152
+    assert tgt.attrs["max_threads_per_block"] == 1024
+    assert tgt.attrs["thread_warp_size"] == 32
+    assert tgt.attrs["registers_per_block"] == 32768
+    assert tgt.host.kind.name == "llvm"
+
+
+def test_target_host_single_dict():
+    tgt = tvm.target.Target({"kind": "llvm", "host": "nvidia/jetson-nano"})
+    assert tgt.kind.name == "llvm"
+    assert tgt.host.kind.name == "cuda"
+    assert tgt.host.attrs["arch"] == "sm_53"
+    assert tgt.host.attrs["shared_memory_per_block"] == 49152
+    assert tgt.host.attrs["max_threads_per_block"] == 1024
+    assert tgt.host.attrs["thread_warp_size"] == 32
+    assert tgt.host.attrs["registers_per_block"] == 32768
+
+
+def test_target_host_single_string():
+    tgt = tvm.target.Target("cuda --host llvm")

Review comment:
       (putting this comment here, but this is not comment about this test, 
just wanted to pin it somewhere with a reference)
   
   I noticed there are some syntax changes in this. If this becomes (with this 
PR) our preferred way to declare the "target to target host" dependency, should 
we also update the tutorials to reflect that?

##########
File path: include/tvm/target/target_kind.h
##########
@@ -376,7 +376,8 @@ inline TargetKindRegEntry& TargetKindRegEntry::set_name() {
           .add_attr_option<String>("tag")                         \
           .add_attr_option<String>("device")                      \
           .add_attr_option<String>("model")                       \
-          .add_attr_option<Array<String>>("libs")
+          .add_attr_option<Array<String>>("libs")                 \
+          .add_attr_option<Target>("host")

Review comment:
       I'm just curious on how this plays with existing logic in `build()` at 
`build_module.py`, which has a `target_host` parameter:
   
   
https://github.com/apache/tvm/blob/b7e0cfb6d469c3745ae2195908daadea9c64d87e/python/tvm/relay/build_module.py#L197
   
   Does this mean we now have two ways of declaring `target_host`? If so, are 
planning to deprecate one of them?
   
   Also, in case the user declares both (_doesn't make much sense, but can be 
done once this PR is meged_), which one takes precedece, or, should we show an 
error? I'm referrin to something like `relay.build(target="cuda --host 
nvidia/jetson-nano", target_host="llvm"...)`




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


Reply via email to