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/23 06:55:43 UTC
[GitHub] [tvm] jcf94 opened a new pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
jcf94 opened a new pull request #7156:
URL: https://github.com/apache/tvm/pull/7156
AutoScheduler uses a cost model to guide the search search.
We now have NO_REWRITE, INSERT_TRANSFORM_STAGE, REWRITE_FOR_PRE_TRANSFORMED three options when applying schedule from AutoScheduler.
In my tests, if we set REWRITE_FOR_PRE_TRANSFORMED in program measuring, the final schedule we get will trend to perform better in REWRITE_FOR_PRE_TRANSFORMED mode. Though this schedule also works in other options, it will not perform the best performance if we would like to get a kernel with NO_REWRITE.
This PR:
1. Add a layout rewrite option for SearchTask, which will be passed to program measuring.
2. This option will be set to NO_REWRITE in default, and REWRITE_FOR_PRE_TRANSFORMED when working in a end to end network task.
3. Update the schedule for the inserted transform stage in option INSERT_TRANSFORM_STAGE, according to `python/tvm/topi/x86/injective.py`.
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548960303
##########
File path: src/auto_scheduler/measure_record.cc
##########
@@ -165,12 +165,16 @@ struct Handler<::tvm::auto_scheduler::SearchTaskNode> {
writer->WriteArrayItem(*data.hardware_params.get());
if (data.target_host.defined()) {
writer->WriteArrayItem(data.target_host->str());
+ } else {
+ writer->WriteArrayItem(std::string(""));
}
+ writer->WriteArrayItem(static_cast<int>(data.layout_rewrite_option));
Review comment:
bump the version number for this change.
##########
File path: include/tvm/auto_scheduler/search_task.h
##########
@@ -144,9 +147,10 @@ class SearchTask : public ObjectRef {
* \param target The target device of this search task.
* \param target_host The target host device of this search task.
* \param hardware_params Hardware parameters used in this search task.
+ * \param layout_rewrite_option The default layout rewrite option used during program measuring.
Review comment:
```suggestion
* \param layout_rewrite_option The layout rewrite option used for measuring programs.
```
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548959880
##########
File path: python/tvm/auto_scheduler/search_task.py
##########
@@ -227,6 +227,12 @@ def __init__(
if isinstance(target_host, str):
target_host = Target(target_host)
+ if layout_rewrite_option is None:
+ layout_rewrite_option = LayoutRewriteOption.NO_REWRITE
+ if target.kind.name == "llvm" or \
Review comment:
1. move `enable_layout_rewrite` from `python/tvm/auto_scheduler/relay_integration.py` to this file.
2. call `enable_layout_rewrite` function here
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548440939
##########
File path: src/auto_scheduler/compute_dag.cc
##########
@@ -1023,7 +1032,10 @@ ComputeDAG ComputeDAG::RewriteLayout(Array<Step>* transform_steps,
}
original_compute_op = op;
CHECK(!new_compute_op.defined());
- new_compute_op = te::ComputeOp(pop->name, pop->tag, pop->attrs, pop->axis, new_body);
+ auto new_attrs = pop->attrs;
+ new_attrs.Set("ori_placeholder_layout", tvm::String(origin_layout));
+ new_attrs.Set("new_placeholder_layout", tvm::String(new_layout));
+ new_compute_op = te::ComputeOp(pop->name, pop->tag, new_attrs, pop->axis, new_body);
Review comment:
No. `Conv2DAttrs` is used to pass layout information across multiple relay passes.
The object created here cannot live across multiple relay passes.
----------------------------------------------------------------
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 #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548116114
##########
File path: python/tvm/auto_scheduler/compute_dag.py
##########
@@ -32,7 +32,12 @@
class LayoutRewriteOption:
- """Options for applying layout rewrite."""
+ """
+ Options for applying layout rewrite.
+
+ The NO_REWRITE and INSERT_TRANSFORM_STAGE is expected to be used when tuning a dependent op,
+ and the REWRITE_FOR_PRE_TRANSFORMED is expected to be used when tuning ops inside a network.
Review comment:
"dependent op" may be confusing. Could you change to another term that against to "ops inside a network"?
```suggestion
The NO_REWRITE and INSERT_TRANSFORM_STAGE are expected to be used when tuning a dependent op,
and the REWRITE_FOR_PRE_TRANSFORMED is expected to be used when tuning ops inside a network.
```
##########
File path: python/tvm/auto_scheduler/search_task.py
##########
@@ -245,15 +254,18 @@ def tune(self, tuning_options, search_policy=None):
_ffi_api.AutoSchedule(search_policy, tuning_options)
- def apply_best(self, log_file, layout_rewrite_option=None):
+ def apply_best(self, log_file, layout_rewrite_option=LayoutRewriteOption.NO_REWRITE):
Review comment:
Out of scope: Is it possible to maintain the layout rewrite option in the record? This semantic indicates that you need to know the right layout rewrite option when you want to apply a tuned log.
##########
File path: python/tvm/auto_scheduler/search_task.py
##########
@@ -178,6 +178,12 @@ class SearchTask(Object):
The target host device of this search task.
hardware_params : Optional[HardwareParams]
Hardware parameters used in this search task.
+ layout_rewrite_option : LayoutRewriteOption = LayoutRewriteOption.NO_REWRITE
+ The default layout rewrite option used during program measuring.
+ Cost model will adjust the auto scheduler to find a better schedule for the specified
+ layout rewrite option.
Review comment:
I think we can directly say "auto_scheduler will find a better schedule for the specified layout rewrite option". Cost model guided search is too detail for end users.
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548442671
##########
File path: python/tvm/auto_scheduler/compute_dag.py
##########
@@ -32,7 +32,12 @@
class LayoutRewriteOption:
- """Options for applying layout rewrite."""
+ """
+ Options for applying layout rewrite.
+
+ The NO_REWRITE and INSERT_TRANSFORM_STAGE is expected to be used when tuning a dependent op,
+ and the REWRITE_FOR_PRE_TRANSFORMED is expected to be used when tuning ops inside a network.
Review comment:
"a dependent op" -> "an independent op"
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548452026
##########
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, and the tutorial won't perform layout rewrite.
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:
users@infra.apache.org
[GitHub] [tvm] merrymercy commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548456207
##########
File path: python/tvm/auto_scheduler/relay_integration.py
##########
@@ -126,6 +126,9 @@ def extract_tasks(
target=target,
target_host=target_host,
hardware_params=hardware_params,
+ # When auto scheduler is used in end to end network, try to apply layout rewrite
+ # to improve the overall performance
+ layout_rewrite_option=LayoutRewriteOption.REWRITE_FOR_PRE_TRANSFORMED
Review comment:
We can set layout rewrite options according to the target.
For cpu/mali, we can use `REWRITE_FOR_PRE_TRANSFORMED`. For cuda, we can use `NO_REWRITE`.
We can create a common utility function to reuse these lines.
https://github.com/apache/tvm/blob/0b2ab56705a1c26aef402d1c0d48a887fc76dbd3/python/tvm/auto_scheduler/relay_integration.py#L262-L268
The current code is correct because some condition in `ComputeDAG::LayoutRewrite` just does not hold for CUDA schedules. But this is implicit and very confusing to new users.
We can add this change makes it more clear.
----------------------------------------------------------------
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] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548956205
##########
File path: python/tvm/auto_scheduler/search_task.py
##########
@@ -178,6 +178,12 @@ class SearchTask(Object):
The target host device of this search task.
hardware_params : Optional[HardwareParams]
Hardware parameters used in this search task.
+ layout_rewrite_option : Optional[LayoutRewriteOption]
+ The default layout rewrite option used during program measuring.
Review comment:
Thanks, fixed.
----------------------------------------------------------------
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] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548517233
##########
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:
The problem seems not only about the compatability.
The code above:
https://github.com/apache/tvm/blob/0b2ab56705a1c26aef402d1c0d48a887fc76dbd3/src/auto_scheduler/measure_record.cc#L166-L168
allows different log format even when the log version is all v0.4.
----------------------------------------------------------------
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] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548513446
##########
File path: python/tvm/auto_scheduler/search_task.py
##########
@@ -245,15 +254,18 @@ def tune(self, tuning_options, search_policy=None):
_ffi_api.AutoSchedule(search_policy, tuning_options)
- def apply_best(self, log_file, layout_rewrite_option=None):
+ def apply_best(self, log_file, layout_rewrite_option=LayoutRewriteOption.NO_REWRITE):
Review comment:
Emm ... Then we should also add this to maybe MeasureInput? I'm thinking this may not be so necessary, since we'll always try to apply layout rewrite on network, and this API may only be used in the replying of standalone ops. The user of this API should know what this means.
----------------------------------------------------------------
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] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548953235
##########
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:
Done.
##########
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:
Done.
----------------------------------------------------------------
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] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548965297
##########
File path: python/tvm/auto_scheduler/search_task.py
##########
@@ -220,8 +228,21 @@ def __init__(
if isinstance(target_host, str):
target_host = Target(target_host)
+ if layout_rewrite_option is None:
+ layout_rewrite_option = LayoutRewriteOption.NO_REWRITE
Review comment:
I moved this function to LayoutRewriteOption.get_targer_default.
----------------------------------------------------------------
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] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548517233
##########
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:
The problem seems not only about the compatability.
The code above:
https://github.com/apache/tvm/blob/0b2ab56705a1c26aef402d1c0d48a887fc76dbd3/src/auto_scheduler/measure_record.cc#L166-L168
allows different log format even when the log version is v0.4.
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548456207
##########
File path: python/tvm/auto_scheduler/relay_integration.py
##########
@@ -126,6 +126,9 @@ def extract_tasks(
target=target,
target_host=target_host,
hardware_params=hardware_params,
+ # When auto scheduler is used in end to end network, try to apply layout rewrite
+ # to improve the overall performance
+ layout_rewrite_option=LayoutRewriteOption.REWRITE_FOR_PRE_TRANSFORMED
Review comment:
We can set layout rewrite options according to the target.
For cpu/mali, we can use `REWRITE_FOR_PRE_TRANSFORMED`. For cuda, we can use `NO_REWRITE`.
We can create a common utility function to reuse these lines.
https://github.com/apache/tvm/blob/0b2ab56705a1c26aef402d1c0d48a887fc76dbd3/python/tvm/auto_scheduler/relay_integration.py#L262-L268
----------------------------------------------------------------
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 #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548954743
##########
File path: python/tvm/auto_scheduler/search_task.py
##########
@@ -178,6 +178,12 @@ class SearchTask(Object):
The target host device of this search task.
hardware_params : Optional[HardwareParams]
Hardware parameters used in this search task.
+ layout_rewrite_option : Optional[LayoutRewriteOption]
+ The default layout rewrite option used during program measuring.
Review comment:
```suggestion
The layout rewrite option used during program measuring. If None, then XXX will be used.
```
----------------------------------------------------------------
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] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548440354
##########
File path: src/auto_scheduler/compute_dag.cc
##########
@@ -1023,7 +1032,10 @@ ComputeDAG ComputeDAG::RewriteLayout(Array<Step>* transform_steps,
}
original_compute_op = op;
CHECK(!new_compute_op.defined());
- new_compute_op = te::ComputeOp(pop->name, pop->tag, pop->attrs, pop->axis, new_body);
+ auto new_attrs = pop->attrs;
+ new_attrs.Set("ori_placeholder_layout", tvm::String(origin_layout));
+ new_attrs.Set("new_placeholder_layout", tvm::String(new_layout));
+ new_compute_op = te::ComputeOp(pop->name, pop->tag, new_attrs, pop->axis, new_body);
Review comment:
I'm finding that we can get the updated layout information here, can this help us not to add extra things to structures like `Conv2DAttrs`? @merrymercy
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548456207
##########
File path: python/tvm/auto_scheduler/relay_integration.py
##########
@@ -126,6 +126,9 @@ def extract_tasks(
target=target,
target_host=target_host,
hardware_params=hardware_params,
+ # When auto scheduler is used in end to end network, try to apply layout rewrite
+ # to improve the overall performance
+ layout_rewrite_option=LayoutRewriteOption.REWRITE_FOR_PRE_TRANSFORMED
Review comment:
We can set layout rewrite options according to the target.
For cpu/mali, we can use `REWRITE_FOR_PRE_TRANSFORMED`. For cuda, we can use `NO_REWRITE`.
We can create a common utility function to reuse these lines.
https://github.com/apache/tvm/blob/0b2ab56705a1c26aef402d1c0d48a887fc76dbd3/python/tvm/auto_scheduler/relay_integration.py#L262-L268
The current code is correct because some conditions in `ComputeDAG::LayoutRewrite` just do not hold for CUDA schedules. But this is implicit and very confusing to new users.
We can add this change makes the selection of layout rewrite strategy more clear.
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548525776
##########
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:
I see. We should fix this by
```
if (data.target_host.defined()) {
writer->WriteArrayItem(data.target_host->str());
} else {
writer->WriteArrayItem("");
}
```
We can make sure that new writer always writes all fields into the file. But the new reader can read old logs with some fileds missing.
----------------------------------------------------------------
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] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548953251
##########
File path: python/tvm/auto_scheduler/relay_integration.py
##########
@@ -126,6 +126,9 @@ def extract_tasks(
target=target,
target_host=target_host,
hardware_params=hardware_params,
+ # When auto scheduler is used in end to end network, try to apply layout rewrite
+ # to improve the overall performance
+ layout_rewrite_option=LayoutRewriteOption.REWRITE_FOR_PRE_TRANSFORMED
Review comment:
Done.
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548960042
##########
File path: python/tvm/auto_scheduler/search_task.py
##########
@@ -220,8 +228,21 @@ def __init__(
if isinstance(target_host, str):
target_host = Target(target_host)
+ if layout_rewrite_option is None:
+ layout_rewrite_option = LayoutRewriteOption.NO_REWRITE
Review comment:
1. move `enable_layout_rewrite` from `python/tvm/auto_scheduler/relay_integration.py` to this file.
2. call `enable_layout_rewrite` function here
##########
File path: src/auto_scheduler/compute_dag.cc
##########
@@ -1023,7 +1032,10 @@ ComputeDAG ComputeDAG::RewriteLayout(Array<Step>* transform_steps,
}
original_compute_op = op;
CHECK(!new_compute_op.defined());
- new_compute_op = te::ComputeOp(pop->name, pop->tag, pop->attrs, pop->axis, new_body);
+ auto new_attrs = pop->attrs;
+ new_attrs.Set("ori_placeholder_layout", tvm::String(origin_layout));
+ new_attrs.Set("new_placeholder_layout", tvm::String(new_layout));
Review comment:
Delete 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] [tvm] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548965215
##########
File path: src/auto_scheduler/measure_record.cc
##########
@@ -165,12 +165,16 @@ struct Handler<::tvm::auto_scheduler::SearchTaskNode> {
writer->WriteArrayItem(*data.hardware_params.get());
if (data.target_host.defined()) {
writer->WriteArrayItem(data.target_host->str());
+ } else {
+ writer->WriteArrayItem(std::string(""));
}
+ writer->WriteArrayItem(static_cast<int>(data.layout_rewrite_option));
Review comment:
Done.
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548966358
##########
File path: src/auto_scheduler/compute_dag.cc
##########
@@ -1023,7 +1032,10 @@ ComputeDAG ComputeDAG::RewriteLayout(Array<Step>* transform_steps,
}
original_compute_op = op;
CHECK(!new_compute_op.defined());
- new_compute_op = te::ComputeOp(pop->name, pop->tag, pop->attrs, pop->axis, new_body);
+ auto new_attrs = pop->attrs;
+ new_attrs.Set("ori_placeholder_layout", tvm::String(origin_layout));
+ new_attrs.Set("new_placeholder_layout", tvm::String(new_layout));
Review comment:
Ok. We can leave them here
----------------------------------------------------------------
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] jcf94 commented on a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548965483
##########
File path: src/auto_scheduler/compute_dag.cc
##########
@@ -1023,7 +1032,10 @@ ComputeDAG ComputeDAG::RewriteLayout(Array<Step>* transform_steps,
}
original_compute_op = op;
CHECK(!new_compute_op.defined());
- new_compute_op = te::ComputeOp(pop->name, pop->tag, pop->attrs, pop->axis, new_body);
+ auto new_attrs = pop->attrs;
+ new_attrs.Set("ori_placeholder_layout", tvm::String(origin_layout));
+ new_attrs.Set("new_placeholder_layout", tvm::String(new_layout));
Review comment:
I would need to get the layout information when exporting the kernel to run in an environment outside the tvm.
Emm... it's also fine to remove them here.
----------------------------------------------------------------
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 #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548450912
##########
File path: python/tvm/auto_scheduler/compute_dag.py
##########
@@ -32,7 +32,12 @@
class LayoutRewriteOption:
- """Options for applying layout rewrite."""
+ """
+ Options for applying layout rewrite.
+
+ The NO_REWRITE and INSERT_TRANSFORM_STAGE is expected to be used when tuning a dependent op,
+ and the REWRITE_FOR_PRE_TRANSFORMED is expected to be used when tuning ops inside a network.
Review comment:
Ah that makes sense...maybe "a standalone op" would be more straightforward?
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#discussion_r548456207
##########
File path: python/tvm/auto_scheduler/relay_integration.py
##########
@@ -126,6 +126,9 @@ def extract_tasks(
target=target,
target_host=target_host,
hardware_params=hardware_params,
+ # When auto scheduler is used in end to end network, try to apply layout rewrite
+ # to improve the overall performance
+ layout_rewrite_option=LayoutRewriteOption.REWRITE_FOR_PRE_TRANSFORMED
Review comment:
We can set layout rewrite options according to the target.
For cpu/mali, we can use `REWRITE_FOR_PRE_TRANSFORMED`. For cuda, we can use `NO_REWRITE`.
We can create a common utility function to reuse these lines.
https://github.com/apache/tvm/blob/0b2ab56705a1c26aef402d1c0d48a887fc76dbd3/python/tvm/auto_scheduler/relay_integration.py#L262-L268
The current code is correct because some conditions in `ComputeDAG::LayoutRewrite` just do not hold for CUDA schedules. But this is implicit and very confusing to new users.
We can add this change makes it more clear.
----------------------------------------------------------------
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 a change in pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [tvm] jcf94 merged pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 merged pull request #7156:
URL: https://github.com/apache/tvm/pull/7156
----------------------------------------------------------------
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] jcf94 commented on pull request #7156: [AutoScheduler] Update layout rewrite option setting for measuring
Posted by GitBox <gi...@apache.org>.
jcf94 commented on pull request #7156:
URL: https://github.com/apache/tvm/pull/7156#issuecomment-749974856
Will update the log version after #7144 since this PR has modified the log structure of SearchTask.
----------------------------------------------------------------
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