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/11/27 06:26:17 UTC
[GitHub] [tvm] merrymercy opened a new pull request #6981: [AutoScheduler] Accelerate feature extraction for winograd
merrymercy opened a new pull request #6981:
URL: https://github.com/apache/tvm/pull/6981
Compiling Winograd conv2d is very slow. This makes the feature extraction in auto-scheduler very slow.
I found the reason is that simplifying the const matrix expressions in Winograd conv2d takes a lot of time. One example of these expressions is
```
A(i, j) = select(((floormod(i, 6) == 5) && (floormod(j, 4) == 3)), 1f, select(((floormod(i, 6) == 5) && (floormod(j, 4) == 2)), 0f, select(((floormod(i, 6) == 5) && (floormod(j, 4) == 1)), 0f, select(((floormod(i, 6) == 5) && (floormod(j, 4) == 0)), 0f, select(((floormod(i, 6) == 4) && (floormod(j, 4) == 3)), -8f, select(((floormod(i, 6) == 4) && (floormod(j, 4) == 2)), 4f, select(((floormod(i, 6) == 4) && (floormod(j, 4) == 1)), -2f, select(((floormod(i, 6) == 4) && (floormod(j, 4) == 0)), 1f, select(((floormod(i, 6) == 3) && (floormod(j, 4) == 3)), 0.125f, select(((floormod(i, 6) == 3) && (floormod(j, 4) == 2)), 0.25f, select(((floormod(i, 6) == 3) && (floormod(j, 4) == 1)), 0.5f, select(((floormod(i, 6) == 3) && (floormod(j, 4) == 0)), 1f, select(((floormod(i, 6) == 2) && (floormod(j, 4) == 3)), 1f, select(((floormod(i, 6) == 2) && (floormod(j, 4) == 2)), 1f, select(((floormod(i, 6) == 2) && (floormod(j, 4) == 1)), 1f, select(((floormod(i, 6) == 2) && (floormod(j, 4) == 0)), 1f
, select(((floormod(i, 6) == 1) && (floormod(j, 4) == 3)), -1f, select(((floormod(i, 6) == 1) && (floormod(j, 4) == 2)), 1f, select(((floormod(i, 6) == 1) && (floormod(j, 4) == 1)), -1f, select(((floormod(i, 6) == 1) && (floormod(j, 4) == 0)), 1f, select(((floormod(i, 6) == 0) && (floormod(j, 4) == 3)), 0f, select(((floormod(i, 6) == 0) && (floormod(j, 4) == 2)), 0f, select(((floormod(i, 6) == 0) && (floormod(j, 4) == 1)), 0f, select(((floormod(i, 6) == 0) && (floormod(j, 4) == 0)), 1f, 0f))))))))))))))))))))))))
```
But this part is actually useless for feature extraction. So I added a new flag to replace all accesses to constant matrices by constant value 1.
This flag produces a wrong IR but it is good enough for feature extraction purposes.
This accelerates the GA search from 300s to 150s in p3.2x for a Winograd conv2d.
----------------------------------------------------------------
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 #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6981:
URL: https://github.com/apache/tvm/pull/6981#discussion_r531875689
##########
File path: include/tvm/te/schedule.h
##########
@@ -378,6 +378,18 @@ class Schedule : public ObjectRef {
* \return A normalized schedule, can be same as current one.
*/
Schedule normalize();
+
+ /*!
+ * \brief Normalize the schedule for feature extraction in auto-scheduler.
+ * This is similar to `Schedule::normalize`. But we do aggresive simplification
+ * for faster compilation and feature extraction.
+ * The resulted schedule may be wrong. But it is good enough for feature extraction
+ * purposes.
Review comment:
```suggestion
* \brief Normalize the schedule for feature extraction in auto-scheduler.
* This is similar to `Schedule::normalize`, but we do aggressive simplification
* to the TE compute with const_matrix=True for faster compilation and feature extraction.
* The resulted schedule may be wrong, but it is good enough for feature extraction
* purposes.
```
----------------------------------------------------------------
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 #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
merrymercy commented on pull request #6981:
URL: https://github.com/apache/tvm/pull/6981#issuecomment-734927787
@comaniac I created a new API for my use case of `normalize`.
But I leave the attribute `const_matrix`. This is better than `ignore_feature_extraction` because `const_matrix` is a more fundamental attribute. Other analyses (e.g., `AccessAnalyzer`) can also benefit from this attribute.
@FrozenGene @giuseros Please take another 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] merrymercy commented on a change in pull request #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6981:
URL: https://github.com/apache/tvm/pull/6981#discussion_r531706620
##########
File path: src/auto_scheduler/feature.cc
##########
@@ -669,7 +669,7 @@ class PerStoreFeatureExtractor : public StmtExprVisitor {
math_op_counter(node->value);
std::vector<float> mem_bytes_list;
std::vector<float> compute_ops_list;
- int cur_compute_ops;
+ double cur_compute_ops;
Review comment:
int32 will overflow.
----------------------------------------------------------------
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 merged pull request #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
merrymercy merged pull request #6981:
URL: https://github.com/apache/tvm/pull/6981
----------------------------------------------------------------
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] giuseros commented on pull request #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
giuseros commented on pull request #6981:
URL: https://github.com/apache/tvm/pull/6981#issuecomment-734775501
Hi @merrymercy ,
Very nice improvements! I started studying and using the autoscheduler for the ARM cpu backend, so I am sorry if my comments might be obvious/naive
----------------------------------------------------------------
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] giuseros commented on a change in pull request #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
giuseros commented on a change in pull request #6981:
URL: https://github.com/apache/tvm/pull/6981#discussion_r531713662
##########
File path: src/auto_scheduler/feature.cc
##########
@@ -669,7 +669,7 @@ class PerStoreFeatureExtractor : public StmtExprVisitor {
math_op_counter(node->value);
std::vector<float> mem_bytes_list;
std::vector<float> compute_ops_list;
- int cur_compute_ops;
+ double cur_compute_ops;
Review comment:
What about int64? I don't mind if not, but when I read `double` I thought that it could be a non-integer value (e.g., 0.1234)
----------------------------------------------------------------
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 #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6981:
URL: https://github.com/apache/tvm/pull/6981#discussion_r531706620
##########
File path: src/auto_scheduler/feature.cc
##########
@@ -669,7 +669,7 @@ class PerStoreFeatureExtractor : public StmtExprVisitor {
math_op_counter(node->value);
std::vector<float> mem_bytes_list;
std::vector<float> compute_ops_list;
- int cur_compute_ops;
+ double cur_compute_ops;
Review comment:
I found integer overflow with int32.
----------------------------------------------------------------
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 #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
merrymercy commented on pull request #6981:
URL: https://github.com/apache/tvm/pull/6981#issuecomment-734670026
cc @tqchen @FrozenGene @comaniac
----------------------------------------------------------------
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] giuseros commented on a change in pull request #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
giuseros commented on a change in pull request #6981:
URL: https://github.com/apache/tvm/pull/6981#discussion_r531523344
##########
File path: src/auto_scheduler/feature.cc
##########
@@ -669,7 +669,7 @@ class PerStoreFeatureExtractor : public StmtExprVisitor {
math_op_counter(node->value);
std::vector<float> mem_bytes_list;
std::vector<float> compute_ops_list;
- int cur_compute_ops;
+ double cur_compute_ops;
Review comment:
Why moving this to double?
##########
File path: include/tvm/te/schedule.h
##########
@@ -375,9 +375,11 @@ class Schedule : public ObjectRef {
* Insert necessary RebaseNode to make sure all leaf_iter_vars
* are in form [0, extent)
*
+ * \param feature_extraction_mode Whether to enable feature extraction mode for
+ * fast compilation and feature extraction in autotvm/auto-scheduler.
* \return A normalized schedule, can be same as current one.
*/
- Schedule normalize();
+ Schedule normalize(bool feature_extraction_mode);
Review comment:
I agree with others that a different API would be better. But if we stick with this interface, could we default `feature_extraction_mode` to `False` (in the same way you did for the Python API)?
----------------------------------------------------------------
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 #6981: [AutoScheduler] Accelerate feature extraction for winograd
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #6981:
URL: https://github.com/apache/tvm/pull/6981#issuecomment-734720123
I like the idea in general as it may also benefit other workloads that uses selects, but I have some concerns about the new attribute and flag.
For the `const_matrix` attribute, it might be confusing in the future about what does it mean and why. It might be better to make it more feature extraction specific, such as `ignore_feature_extraction`.
In addition, the `feature_extraction_mode` looks a bit ad-hoc. I would rather having a separate API instead of sharing `normalize` and only use it when extracting feature. This separation can also make sure the extensibility of feature improvements in the future.
----------------------------------------------------------------
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