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/25 07:58:44 UTC

[GitHub] [tvm] merrymercy opened a new pull request #7168: [AutoScheduler] Add winograd_conv2d and layout rewrite for conv3d

merrymercy opened a new pull request #7168:
URL: https://github.com/apache/tvm/pull/7168


   - Enable winograd_conv2d for cpu targets
   - Add layout rewrite for conv3d


----------------------------------------------------------------
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 #7168: [AutoScheduler] Enable winograd for conv2d and layout rewrite for conv3d

Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7168:
URL: https://github.com/apache/tvm/pull/7168#discussion_r548845290



##########
File path: python/tvm/topi/nn/conv3d.py
##########
@@ -178,7 +207,12 @@ def conv3d_ndhwc(Input, Filter, stride, padding, dilation, out_dtype="float32"):
         ),
         name="Conv3dOutput",
         tag="conv3d_ndhwc",
+        attrs={"layout_free_placeholders": [Filter]},
     )
+
+    if auto_scheduler_rewritten_layout:
+        Output = auto_scheduler.rewrite_compute_body(Output, auto_scheduler_rewritten_layout)
+
     return Output

Review comment:
       The current style is clearer. For a person who do not want to learn auto-scheduler, he/she can simply skip any code under branch `if auto_scheduler_rewritten_layout`.
   In your style, he/she has to check the code of `auto_scheduler.rewrite_compute_body` to know what's happening.




----------------------------------------------------------------
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 #7168: [AutoScheduler] Enable winograd for conv2d and layout rewrite for conv3d

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #7168:
URL: https://github.com/apache/tvm/pull/7168#discussion_r548888569



##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -476,3 +522,30 @@ def scatter_nd_strategy_cpu(attrs, inputs, out_type, target):
         plevel=10,
     )
     return strategy
+
+
+@conv2d_winograd_without_weight_transfrom_strategy.register("cpu")
+def conv2d_winograd_without_weight_transfrom_strategy_cpu(attrs, inputs, out_type, target):
+    """conv2d_winograd_without_weight_transfrom arm cpu strategy"""

Review comment:
       "arm cpu" -> "cpu"

##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -125,6 +126,32 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
                 wrap_topi_schedule(topi.x86.schedule_conv2d_nhwc),
                 name="conv2d_nhwc.x86",
             )
+
+            judge_winograd_auto_scheduler = False
+            if len(kernel.shape) == 4:
+                kernel_h, kernel_w, _, co = get_const_tuple(kernel.shape)
+                judge_winograd_auto_scheduler = (
+                    "float" in data.dtype
+                    and "float" in kernel.dtype
+                    and kernel_h == 3
+                    and kernel_w == 3
+                    and stride_h == 1
+                    and stride_w == 1
+                    and dilation_h == 1
+                    and dilation_w == 1
+                    and 64 < co < 512

Review comment:
       I think we could add one comment that we find we only work more effective for `64 < co < 512` of resnet18 network on our skylake machine. But this could prompt others maybe you could adjust this condition on your target machine. Otherwise this will bring in some confuse why there is such one condition 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 pull request #7168: [AutoScheduler] Enable winograd for conv2d and layout rewrite for conv3d

Posted by GitBox <gi...@apache.org>.
merrymercy commented on pull request #7168:
URL: https://github.com/apache/tvm/pull/7168#issuecomment-751308421


   @FrozenGene  comments are resolved


----------------------------------------------------------------
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 #7168: [AutoScheduler] Enable winograd for conv2d and layout rewrite for conv3d

Posted by GitBox <gi...@apache.org>.
merrymercy merged pull request #7168:
URL: https://github.com/apache/tvm/pull/7168


   


----------------------------------------------------------------
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 #7168: [AutoScheduler] Enable winograd for conv2d and layout rewrite for conv3d

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #7168:
URL: https://github.com/apache/tvm/pull/7168#discussion_r548888876



##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -125,6 +126,32 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
                 wrap_topi_schedule(topi.x86.schedule_conv2d_nhwc),
                 name="conv2d_nhwc.x86",
             )
+
+            judge_winograd_auto_scheduler = False
+            if len(kernel.shape) == 4:
+                kernel_h, kernel_w, _, co = get_const_tuple(kernel.shape)
+                judge_winograd_auto_scheduler = (
+                    "float" in data.dtype
+                    and "float" in kernel.dtype
+                    and kernel_h == 3
+                    and kernel_w == 3
+                    and stride_h == 1
+                    and stride_w == 1
+                    and dilation_h == 1
+                    and dilation_w == 1
+                    and 64 < co < 512

Review comment:
       I think we could add one comment that we find we only work more effective for `64 < co < 512` of resnet18 network on our skylake machine. But this comment could prompt others maybe you could adjust this condition on your target machine. Otherwise this will bring in some confuse why there is such one condition 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 #7168: [AutoScheduler] Enable winograd for conv2d and layout rewrite for conv3d

Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #7168:
URL: https://github.com/apache/tvm/pull/7168#discussion_r548834418



##########
File path: python/tvm/topi/nn/conv3d.py
##########
@@ -178,7 +207,12 @@ def conv3d_ndhwc(Input, Filter, stride, padding, dilation, out_dtype="float32"):
         ),
         name="Conv3dOutput",
         tag="conv3d_ndhwc",
+        attrs={"layout_free_placeholders": [Filter]},
     )
+
+    if auto_scheduler_rewritten_layout:
+        Output = auto_scheduler.rewrite_compute_body(Output, auto_scheduler_rewritten_layout)
+
     return Output

Review comment:
       How about we move the `auto_scheduler_rewritten_layout` to be checked inside the `rewrite_compute_body`?
   If it is empty, return the original compute.
   ```suggestion
       return auto_scheduler.rewrite_compute_body(Output, auto_scheduler_rewritten_layout)
   ```




----------------------------------------------------------------
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 #7168: [AutoScheduler] Enable winograd for conv2d and layout rewrite for conv3d

Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7168:
URL: https://github.com/apache/tvm/pull/7168#discussion_r548845290



##########
File path: python/tvm/topi/nn/conv3d.py
##########
@@ -178,7 +207,12 @@ def conv3d_ndhwc(Input, Filter, stride, padding, dilation, out_dtype="float32"):
         ),
         name="Conv3dOutput",
         tag="conv3d_ndhwc",
+        attrs={"layout_free_placeholders": [Filter]},
     )
+
+    if auto_scheduler_rewritten_layout:
+        Output = auto_scheduler.rewrite_compute_body(Output, auto_scheduler_rewritten_layout)
+
     return Output

Review comment:
       The current style is clearer. For a person who do not want to learn auto-scheduler, he/she can simply skip any code under branch `if auto_scheduler_rewritten_layout`.
   However, in your style, he/she has to check the code of `auto_scheduler.rewrite_compute_body` to know what's happening, which is an extra burden.




----------------------------------------------------------------
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 #7168: [AutoScheduler] Enable winograd for conv2d and layout rewrite for conv3d

Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7168:
URL: https://github.com/apache/tvm/pull/7168#discussion_r548845290



##########
File path: python/tvm/topi/nn/conv3d.py
##########
@@ -178,7 +207,12 @@ def conv3d_ndhwc(Input, Filter, stride, padding, dilation, out_dtype="float32"):
         ),
         name="Conv3dOutput",
         tag="conv3d_ndhwc",
+        attrs={"layout_free_placeholders": [Filter]},
     )
+
+    if auto_scheduler_rewritten_layout:
+        Output = auto_scheduler.rewrite_compute_body(Output, auto_scheduler_rewritten_layout)
+
     return Output

Review comment:
       The current style is clearer. For a person who do not want to learn auto-scheduler, he/she can simply skip any code under branch `if auto_scheduler_rewritten_layout`.
   However, in your style, he/she has to check the code of `auto_scheduler.rewrite_compute_body` to know the whole logic, which is an extra burden.




----------------------------------------------------------------
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