merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548449738
##########
File path: src/auto_scheduler/measure_record.cc
##########
@@ -182,6 +184,10 @@ struct Handler<::tvm::auto_scheduler::SearchTaskNode> {
reader->Read(&str_value);
data->target = ::tvm::Target(str_value);
s = reader->NextArrayItem();
+ ICHECK(s);
+ reader->Read(&int_value);
+ data->layout_rewrite_option =
::tvm::auto_scheduler::LayoutRewriteOption(int_value);
+ s = reader->NextArrayItem();
Review comment:
Why cannot we make it optional following the pattern below?
Ideally, we should be able to do parsing according to the version number. It
is easy to do this with python's json library, but it seems it is not easy to
do this with the our json parser from dmlc-core. Because we write the version
number as the last field, we cannot get it when parsing the fileds before it.
If we cannot keep backward compatability, we should issue a waring message
and provide an upgrade script.
##########
File path: python/tvm/auto_scheduler/search_task.py
##########
@@ -265,10 +277,6 @@ def apply_best(self, log_file, layout_rewrite_option=None):
"Cannot find any valid schedule for %s in file %s" %
(self.workload_key, log_file)
)
- if layout_rewrite_option is None:
- layout_rewrite_option = LayoutRewriteOption.NO_REWRITE
- if self.target.kind.name == "llvm":
- layout_rewrite_option =
LayoutRewriteOption.INSERT_TRANSFORM_STAGE
Review comment:
This change breaks the current behavior of our tutorial.
In the single op tutorial, we want to be able to control the layout rewrite
optimization with just one line
https://github.com/apache/tvm/blob/0b2ab56705a1c26aef402d1c0d48a887fc76dbd3/tutorials/auto_scheduler/tune_matmul_x86.py#L63
With this attrs, we should do layout rewrite by inserting a new stage.
Without this attr, we should not do layout rewrite.
But after your change, we need to control the layout rewrite in two places.
I propose the following changes:
1. set the default value of `layout_rewrite_option` in `apply_best` as
`None`. If it is `None`, use `task.layout_rewrite_option` in this function.
2. set the default value of `layout_rewrite_option` in `SearchTask` as
'None'. If the target is cpu/mali, use `INSERT_TRANSFORM_STAGE`. If the target
is cuda, use `NO_REWRITE`
----------------------------------------------------------------
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]
