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/12/07 11:16:52 UTC
[GitHub] [tvm] FrozenGene opened a new pull request #7046: [Auto Scheduler] Add target host to measure record
FrozenGene opened a new pull request #7046:
URL: https://github.com/apache/tvm/pull/7046
We don't append target host to measure record serialization / deserialization currently, which works well on x86 host machine / target is simply arm cpu. However, it will have problem for more complex deployment. Say we want to on our x86 server machine cross compile for arm machine's mali GPU, whose target host is arm cpu, but target is mali GPU.
So we should add target host to measure record if `target_host` is not `nullptr`.
----------------------------------------------------------------
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] FrozenGene commented on pull request #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on pull request #7046:
URL: https://github.com/apache/tvm/pull/7046#issuecomment-740321677
@merrymercy @comaniac Have another round of look
----------------------------------------------------------------
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] FrozenGene merged pull request #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
FrozenGene merged pull request #7046:
URL: https://github.com/apache/tvm/pull/7046
----------------------------------------------------------------
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 #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7046:
URL: https://github.com/apache/tvm/pull/7046#issuecomment-740197227
> @comaniac This change does not break compatibility. It can correctly read all old logs. I don't think we have to update the logs.
Ah I didn't notice that we didn't check the log format when reading from file. Should we have that check?
----------------------------------------------------------------
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] merrymercy edited a comment on pull request #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
merrymercy edited a comment on pull request #7046:
URL: https://github.com/apache/tvm/pull/7046#issuecomment-740232724
Currently, we have 4 versions of log format. They all have backward compatibility.
We can call this PR v0.4. This PR can correctly read v0.3 and v0.2. We do not need to do any additional checks.
----------------------------------------------------------------
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 #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7046:
URL: https://github.com/apache/tvm/pull/7046#discussion_r537759346
##########
File path: src/auto_scheduler/measure_record.cc
##########
@@ -183,7 +186,12 @@ struct Handler<::tvm::auto_scheduler::SearchTaskNode> {
reader->Read(hardware_params_node.get());
s = reader->NextArrayItem();
data->hardware_params = ::tvm::auto_scheduler::HardwareParams(hardware_params_node);
- ICHECK(!s);
+ if (s) {
+ reader->Read(&str_value);
+ data->target_host = ::tvm::Target(str_value);
+ s = reader->NextArrayItem();
+ ICHECK(!s);
Review comment:
This check should be out of this if-statement.
----------------------------------------------------------------
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] FrozenGene commented on a change in pull request #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #7046:
URL: https://github.com/apache/tvm/pull/7046#discussion_r537987159
##########
File path: src/auto_scheduler/measure_record.cc
##########
@@ -183,7 +186,12 @@ struct Handler<::tvm::auto_scheduler::SearchTaskNode> {
reader->Read(hardware_params_node.get());
s = reader->NextArrayItem();
data->hardware_params = ::tvm::auto_scheduler::HardwareParams(hardware_params_node);
- ICHECK(!s);
+ if (s) {
+ reader->Read(&str_value);
+ data->target_host = ::tvm::Target(str_value);
+ s = reader->NextArrayItem();
+ ICHECK(!s);
Review comment:
Move out will break back compatibility
----------------------------------------------------------------
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] FrozenGene commented on pull request #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on pull request #7046:
URL: https://github.com/apache/tvm/pull/7046#issuecomment-739853283
@merrymercy @comaniac @jcf94 @minminsun
----------------------------------------------------------------
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] merrymercy commented on pull request #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
merrymercy commented on pull request #7046:
URL: https://github.com/apache/tvm/pull/7046#issuecomment-740195089
@comaniac This change does not break compatibility. It can correctly read all old logs. I don't think we have to update the logs.
----------------------------------------------------------------
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 #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7046:
URL: https://github.com/apache/tvm/pull/7046#issuecomment-740235434
> Currently, we have 4 versions of log format. They all have backward compatibility.
> We can call this PR v0.4. This PR can correctly read v0.3 and v0.2. We do not need to do any additional checks.
Yeah that's definitely more flexible. I'm just afraid that it might introduce some confusions, as AutoTVM strictly checks log versions. Maybe we could let it be for now and add the check once the newer log format is no longer backward compatible.
----------------------------------------------------------------
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] merrymercy commented on pull request #7046: [Auto Scheduler] Add target host to measure record
Posted by GitBox <gi...@apache.org>.
merrymercy commented on pull request #7046:
URL: https://github.com/apache/tvm/pull/7046#issuecomment-740232724
Currently, we have 4 versions of log format. They all have backward compatibility.
We can call this PR v0.4. v0.4 can correctly read v0.3 and v0.2. We do not need to do any additional checks.
----------------------------------------------------------------
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