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 2021/06/02 14:35:06 UTC
[GitHub] [tvm] leandron opened a new pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
leandron opened a new pull request #8176:
URL: https://github.com/apache/tvm/pull/8176
Enable `tvmc compile --target "opencl, llvm" ...` as a valid target, updates some validations and add test cases.
* Removes validation to accept up to two TVM targets and set them as target and target_host
--
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] [tvm] comaniac commented on a change in pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#discussion_r644401547
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -295,10 +299,20 @@ def target_from_cli(target):
raise TVMCException(f"Error parsing target string '{target}'.\nThe error was: {ex}")
validate_targets(parsed_targets)
- target = parsed_targets[-1]["raw"]
- extra_targets = parsed_targets[:-1] if len(parsed_targets) > 1 else []
+ tvm_targets = [t for t in parsed_targets if t["is_tvm_target"]]
+
+ # Validated target strings have 1 or 2 tvm targets, otherwise
+ # `validate_targets` above will fail.
+ if len(tvm_targets) == 1:
+ target = tvm_targets[0]["raw"]
+ target_host = None
+ elif len(tvm_targets) == 2:
Review comment:
I know there already has a checker before invoking this function, but it would be better to use assert to make our purpose clear.
```suggestion
else:
assert len(tvm_targets) == 2
```
--
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] [tvm] comaniac commented on pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#issuecomment-854001678
Thanks @leandron @gromero.
--
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] [tvm] gromero edited a comment on pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
gromero edited a comment on pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#issuecomment-853376978
I'm not a reviewer, but I took a look and it LGTM. Cheers.
--
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] [tvm] leandron commented on a change in pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#discussion_r644609359
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -295,10 +299,20 @@ def target_from_cli(target):
raise TVMCException(f"Error parsing target string '{target}'.\nThe error was: {ex}")
validate_targets(parsed_targets)
- target = parsed_targets[-1]["raw"]
- extra_targets = parsed_targets[:-1] if len(parsed_targets) > 1 else []
+ tvm_targets = [t for t in parsed_targets if t["is_tvm_target"]]
+
+ # Validated target strings have 1 or 2 tvm targets, otherwise
+ # `validate_targets` above will fail.
+ if len(tvm_targets) == 1:
+ target = tvm_targets[0]["raw"]
+ target_host = None
+ elif len(tvm_targets) == 2:
Review comment:
Yeah, makes sense.
--
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] [tvm] comaniac commented on pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#issuecomment-854001678
Thanks @leandron @gromero.
--
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] [tvm] comaniac merged pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
comaniac merged pull request #8176:
URL: https://github.com/apache/tvm/pull/8176
--
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] [tvm] leandron commented on a change in pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#discussion_r644609359
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -295,10 +299,20 @@ def target_from_cli(target):
raise TVMCException(f"Error parsing target string '{target}'.\nThe error was: {ex}")
validate_targets(parsed_targets)
- target = parsed_targets[-1]["raw"]
- extra_targets = parsed_targets[:-1] if len(parsed_targets) > 1 else []
+ tvm_targets = [t for t in parsed_targets if t["is_tvm_target"]]
+
+ # Validated target strings have 1 or 2 tvm targets, otherwise
+ # `validate_targets` above will fail.
+ if len(tvm_targets) == 1:
+ target = tvm_targets[0]["raw"]
+ target_host = None
+ elif len(tvm_targets) == 2:
Review comment:
Yeah, makes sense.
--
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] [tvm] comaniac commented on a change in pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#discussion_r644401547
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -295,10 +299,20 @@ def target_from_cli(target):
raise TVMCException(f"Error parsing target string '{target}'.\nThe error was: {ex}")
validate_targets(parsed_targets)
- target = parsed_targets[-1]["raw"]
- extra_targets = parsed_targets[:-1] if len(parsed_targets) > 1 else []
+ tvm_targets = [t for t in parsed_targets if t["is_tvm_target"]]
+
+ # Validated target strings have 1 or 2 tvm targets, otherwise
+ # `validate_targets` above will fail.
+ if len(tvm_targets) == 1:
+ target = tvm_targets[0]["raw"]
+ target_host = None
+ elif len(tvm_targets) == 2:
Review comment:
I know there already has a checker before invoking this function, but it would be better to use assert to make our purpose clear.
```suggestion
else:
assert len(tvm_targets) == 2
```
--
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] [tvm] leandron commented on pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#issuecomment-853084186
cc @gromero @jwfromm @comaniac for reviews
--
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] [tvm] gromero commented on pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#issuecomment-853376978
I'm not a reviewer, but I've took a look and it LGTM. Cheers.
--
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] [tvm] comaniac merged pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
comaniac merged pull request #8176:
URL: https://github.com/apache/tvm/pull/8176
--
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] [tvm] gromero edited a comment on pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
gromero edited a comment on pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#issuecomment-853376978
I'm not a reviewer, but I took a look and it LGTM. Cheers.
--
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] [tvm] gromero commented on pull request #8176: [TVMC] Fix tvmc compile to extract target and target_host from --target
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #8176:
URL: https://github.com/apache/tvm/pull/8176#issuecomment-853376978
I'm not a reviewer, but I've took a look and it LGTM. Cheers.
--
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