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/04/08 23:06:42 UTC

[GitHub] [incubator-tvm] jwfromm opened a new pull request #5284: [Topi] Tensorcore support for Conv3D

jwfromm opened a new pull request #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284
 
 
   This PR is a pretty direct port of the conv2d tensorcore schedules introduced in [PR #5099](https://github.com/apache/incubator-tvm/pull/5099) to conv3d. In my early testing I've found this new schedule to be up to 10X faster than the default conv3d schedule. I also snuck one little adjustment into the conv3d winograd schedule that helps for smaller workloads.
   

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] jwfromm commented on issue #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
jwfromm commented on issue #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#issuecomment-611239109
 
 
   @Shawn-Inspur, @Laurawly could you take a look at this PR?

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen merged pull request #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284
 
 
   

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#issuecomment-613017061
 
 
   Thanks  @jwfromm @Laurawly @Shawn-Inspur  @yangjunpro @icemelon9 !

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] icemelon9 commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#discussion_r407151713
 
 

 ##########
 File path: python/tvm/relay/op/nn/_nn.py
 ##########
 @@ -139,13 +139,17 @@ def convert_conv2d(attrs, inputs, tinfos, desired_layout):
     # pylint: disable=import-outside-toplevel
     from tvm import relay
     data, weight = inputs
-    assert desired_layout == 'NCHW', \
-            "Currently only transformation to NCHW layout is supported."
+    new_attrs = dict(attrs)
     if desired_layout == 'NCHW':
-        new_attrs = dict(attrs)
         new_attrs['data_layout'] = desired_layout
         new_attrs['kernel_layout'] = 'OIHW'
         return relay.nn.conv2d(data, weight, **new_attrs)
+    elif desired_layout == 'NHWC':
+        new_attrs['data_layout'] = desired_layout
+        new_attrs['kernel_layout'] = 'HWIO'
 
 Review comment:
   This could be wrong for depthwise conv2d, which by default expects kernel layout to be "HWOI".

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] yangjunpro commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
yangjunpro commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#discussion_r407191106
 
 

 ##########
 File path: python/tvm/relay/op/nn/_nn.py
 ##########
 @@ -183,6 +193,42 @@ def alter_op_layout_conv3d(attrs, inputs, tinfos, out_type):
     """Alternate the layout of conv3d"""
     return topi.nn.conv3d_alter_layout(attrs, inputs, tinfos, out_type)
 
+@reg.register_convert_op_layout("nn.conv3d")
+def convert_conv3d(attrs, inputs, tinfos, desired_layout):
+    """Convert Layout pass registration for conv3d op.
+
+    Parameters
+    ----------
+    attrs : tvm.ir.Attrs
+        Attributes of current convolution
+    inputs : list of tvm.relay.Expr
+        The args of the Relay expr to be legalized
+    tinfos : list of types
+        List of input and output types
+    desired_layout : str
+        The desired layout
+
+    Returns
+    -------
+    result : tvm.relay.Expr
+        The transformed expr
+    """
+    # pylint: disable=import-outside-toplevel
+    from tvm import relay
+    data, weight = inputs
+    new_attrs = dict(attrs)
+    if desired_layout == 'NCDHW':
+        new_attrs['data_layout'] = desired_layout
 
 Review comment:
   Better to move the common attribute assignment out of the if/elif branches.

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] FrozenGene commented on issue #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on issue #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#issuecomment-611427467
 
 
   @yangjunpro @minminsun Please also have a look, because you have done tensorcore support ever.

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] jwfromm commented on issue #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
jwfromm commented on issue #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#issuecomment-611259778
 
 
   Anyone know what the deal with the CI failure is? Seems to work fine on my local branch and has nothing to do with this PR.
   ```
   Running './tests/scripts/task_sphinx_precheck.sh' inside tvmai/ci-gpu:v0.61...
   mesg: ttyname failed: Inappropriate ioctl for device
   Adding group `tvm' (GID 1000) ...
   Done.
   PreCheck sphinx doc generation WARNINGS..
   rm -rf _build/*
   rm -rf gen_modules
   rm -rf tutorials
   rm -rf vta/tutorials
   python3 -m sphinx -b html -d _build/doctrees   . _build/html
   Running Sphinx v2.4.1
   Makefile:70: recipe for target 'html' failed
   script returned exit code 2
   ```

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] jwfromm commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#discussion_r406608704
 
 

 ##########
 File path: python/tvm/relay/op/strategy/cuda.py
 ##########
 @@ -268,11 +268,24 @@ def conv3d_strategy_cuda(attrs, inputs, out_type, target):
                 wrap_topi_schedule(topi.cuda.schedule_conv3d_ncdhw_winograd),
                 name="conv3d_ncdhw_winograd.cuda",
                 plevel=5)
-    else: # layout == "NDHWC":
-        strategy.add_implementation(wrap_compute_conv3d(topi.cuda.conv3d_ndhwc),
-                                    wrap_topi_schedule(topi.cuda.schedule_conv3d_ndhwc),
-                                    name="conv3d_ndhwc.cuda",
-                                    plevel=10)
+    else:  # layout == "NDHWC":
+        strategy.add_implementation(
+            wrap_compute_conv3d(topi.cuda.conv3d_ndhwc),
+            wrap_topi_schedule(topi.cuda.schedule_conv3d_ndhwc),
+            name="conv3d_ndhwc.cuda",
+            plevel=10)
+        N, _, _, _, _ = get_const_tuple(data.shape)
+        _, _, _, CI, CO = get_const_tuple(kernel.shape)
+        if nvcc.have_tensorcore(tvm.gpu(0).compute_version):
 
 Review comment:
   Added check for cuda to both conv2d and conv3d strategy call.

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] icemelon9 commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#discussion_r407151248
 
 

 ##########
 File path: topi/tests/python/test_topi_conv3d_ndhwc_tensorcore.py
 ##########
 @@ -0,0 +1,127 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name, too-many-locals, too-many-arguments
+"""Example code to do convolution."""
+
+import numpy as np
+import tvm
+import topi
+import topi.testing
+from tvm import te
+from tvm.contrib.pickle_memoize import memoize
+from tvm.contrib import nvcc
+from topi.nn.util import get_pad_tuple3d
+from topi.util import get_const_tuple
+
+
+_conv3d_ndhwc_tensorcore_implement = {
+    "cuda": (topi.cuda.conv3d_ndhwc_tensorcore, topi.cuda.schedule_conv3d_ndhwc_tensorcore)
+}
+
+
+def verify_conv3d_ndhwc(batch, in_channel, in_size, num_filter, kernel, stride,
+                        padding, dilation=1, add_bias=False, add_relu=False, devices='cuda'):
+    """Test the conv2d with tensorcore for nhwc layout"""
+    pad_front, pad_top, pad_left, pad_back, pad_bottom, pad_right = get_pad_tuple3d(
+        padding, (kernel, kernel, kernel))
+    padding_sum = pad_front + pad_top + pad_left + pad_back + pad_bottom + pad_right
+    print("Workload: (%d, %d, %d, %d, %d, %d, %d, %d)" % (
+        batch, in_channel, in_size, num_filter, kernel, stride, padding_sum, dilation))
+
+    in_depth = in_height = in_width = in_size
+
+    A = te.placeholder((batch, in_depth, in_height, in_width, in_channel), name='A')
+    W = te.placeholder((kernel, kernel, kernel, in_channel, num_filter), name='W')
+    bias = te.placeholder((1, 1, 1, 1, num_filter), name='bias')
+
+    a_shape = get_const_tuple(A.shape)
+    w_shape = get_const_tuple(W.shape)
+    bias_shape = get_const_tuple(bias.shape)
+    dtype = A.dtype
+
+    @memoize("topi.tests.test_topi_conv2d_nhwc.verify_conv2d_nhwc")
 
 Review comment:
   ```suggestion
       @memoize("topi.tests.test_topi_conv3d_ndhwc. verify_conv3d_ndhwc")
   ```

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] Shawn-Inspur commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
Shawn-Inspur commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#discussion_r406157545
 
 

 ##########
 File path: python/tvm/relay/op/strategy/cuda.py
 ##########
 @@ -268,11 +268,24 @@ def conv3d_strategy_cuda(attrs, inputs, out_type, target):
                 wrap_topi_schedule(topi.cuda.schedule_conv3d_ncdhw_winograd),
                 name="conv3d_ncdhw_winograd.cuda",
                 plevel=5)
-    else: # layout == "NDHWC":
-        strategy.add_implementation(wrap_compute_conv3d(topi.cuda.conv3d_ndhwc),
-                                    wrap_topi_schedule(topi.cuda.schedule_conv3d_ndhwc),
-                                    name="conv3d_ndhwc.cuda",
-                                    plevel=10)
+    else:  # layout == "NDHWC":
+        strategy.add_implementation(
+            wrap_compute_conv3d(topi.cuda.conv3d_ndhwc),
+            wrap_topi_schedule(topi.cuda.schedule_conv3d_ndhwc),
+            name="conv3d_ndhwc.cuda",
+            plevel=10)
+        N, _, _, _, _ = get_const_tuple(data.shape)
+        _, _, _, CI, CO = get_const_tuple(kernel.shape)
+        if nvcc.have_tensorcore(tvm.gpu(0).compute_version):
 
 Review comment:
   Could you please check both the target_name and nvcc.have_tensorcore to skip nvcc.have_tensorcore check when CUDA is not available? 
   Please refer to [Unable to get started with Metal on macOS 10.13 with Radeon Pro 560 4096 MB](https://discuss.tvm.ai/t/unable-to-get-started-with-metal-on-macos-10-13-with-radeon-pro-560-4096-mb/6209/14) for details. 

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#issuecomment-611264859
 
 
   The sphinx error is a known flaky case that we should look into, please push to retrigger

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] jwfromm commented on issue #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
jwfromm commented on issue #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#issuecomment-612667056
 
 
   @yangjunpro, funnily enough I asked nearly the same question to the authors of the conv2d tensorcore schedule. You can read their answer[ here](https://github.com/apache/incubator-tvm/pull/5099). The quick take is that it does not support conv (like you mentioned) but more importantly that it causes significant performance regression compared to a bespoke approach like this one.

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#issuecomment-611243277
 
 
   also cc @icemelon9 since it is related to stragey

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] jwfromm commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D

Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #5284: [Topi] Tensorcore support for Conv3D
URL: https://github.com/apache/incubator-tvm/pull/5284#discussion_r407161101
 
 

 ##########
 File path: python/tvm/relay/op/nn/_nn.py
 ##########
 @@ -139,13 +139,17 @@ def convert_conv2d(attrs, inputs, tinfos, desired_layout):
     # pylint: disable=import-outside-toplevel
     from tvm import relay
     data, weight = inputs
-    assert desired_layout == 'NCHW', \
-            "Currently only transformation to NCHW layout is supported."
+    new_attrs = dict(attrs)
     if desired_layout == 'NCHW':
-        new_attrs = dict(attrs)
         new_attrs['data_layout'] = desired_layout
         new_attrs['kernel_layout'] = 'OIHW'
         return relay.nn.conv2d(data, weight, **new_attrs)
+    elif desired_layout == 'NHWC':
+        new_attrs['data_layout'] = desired_layout
+        new_attrs['kernel_layout'] = 'HWIO'
 
 Review comment:
   Good point, I added a depthwise check 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


With regards,
Apache Git Services