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 2022/08/19 04:50:32 UTC

[GitHub] [tvm] junrushao1994 opened a new pull request, #12496: [TVMScript] Reorganize the folder structure

junrushao1994 opened a new pull request, #12496:
URL: https://github.com/apache/tvm/pull/12496

   This PR introduces some minor restructuring of the `python/tvm/script`
   folder structure to make it more convenient for future upstreaming.
   
   CC: @Hzfengsy @cyx-6 


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] junrushao commented on a diff in pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #12496:
URL: https://github.com/apache/tvm/pull/12496#discussion_r1020598148


##########
python/tvm/script/__init__.py:
##########
@@ -14,8 +14,5 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-"""TVM Script APIs of TVM Python Package, aimed to support TIR"""
-
-from . import tir
-
-from .parser import ir_module, from_source
+"""TVM Script APIs of TVM Python Package"""
+from .parser import ir, ir_module, parse as from_source, tir

Review Comment:
   separate by commas, `parse` becomes `from_source` while others are not renamed



##########
python/tvm/script/__init__.py:
##########
@@ -14,8 +14,5 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-"""TVM Script APIs of TVM Python Package, aimed to support TIR"""
-
-from . import tir
-
-from .parser import ir_module, from_source
+"""TVM Script APIs of TVM Python Package"""
+from .parser import ir, ir_module, parse as from_source, tir

Review Comment:
   separated by commas, `parse` becomes `from_source` while others are not renamed



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] junrushao commented on a diff in pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #12496:
URL: https://github.com/apache/tvm/pull/12496#discussion_r1020621395


##########
python/tvm/tir/buffer.py:
##########
@@ -187,13 +187,11 @@ def __getitem__(self, indices):
         if any(isinstance(index, slice) and index.step is None for index in indices):
             region = []
             analyzer = Analyzer()
-            for index in indices:
+            for i, index in zip(range(len(indices)), indices):
                 if isinstance(index, slice):
-                    region.append(
-                        Range.from_min_extent(
-                            index.start, analyzer.simplify(index.stop - index.start)
-                        )
-                    )
+                    start = 0 if index.start is None else index.start
+                    stop = self.shape[i] if index.stop is None else index.stop
+                    region.append(Range.from_min_extent(start, analyzer.simplify(stop - start)))

Review Comment:
   yep we don't allow non-trivial `index.step` 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.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] MasterJH5574 merged pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
MasterJH5574 merged PR #12496:
URL: https://github.com/apache/tvm/pull/12496


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] junrushao commented on a diff in pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #12496:
URL: https://github.com/apache/tvm/pull/12496#discussion_r1020597662


##########
python/tvm/tir/buffer.py:
##########
@@ -187,13 +187,11 @@ def __getitem__(self, indices):
         if any(isinstance(index, slice) and index.step is None for index in indices):
             region = []
             analyzer = Analyzer()
-            for index in indices:
+            for i, index in zip(range(len(indices)), indices):

Review Comment:
   good idea!



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] MasterJH5574 commented on a diff in pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
MasterJH5574 commented on code in PR #12496:
URL: https://github.com/apache/tvm/pull/12496#discussion_r1020665951


##########
python/tvm/script/__init__.py:
##########
@@ -14,8 +14,5 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-"""TVM Script APIs of TVM Python Package, aimed to support TIR"""
-
-from . import tir
-
-from .parser import ir_module, from_source
+"""TVM Script APIs of TVM Python Package"""
+from .parser import ir, ir_module, parse as from_source, tir

Review Comment:
   Oh I see. Thanks for the explanation :-)



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] junrushao1994 commented on pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #12496:
URL: https://github.com/apache/tvm/pull/12496#issuecomment-1220907528

   Okay either way works


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] tvm-bot commented on pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #12496:
URL: https://github.com/apache/tvm/pull/12496#issuecomment-1311312359

   <!---bot-comment-->
   
   Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
   
   <!--bot-comment-docs-start-->
    * Built docs for commit 53ead89bd33ae0e38b82992f32030a1f63ad6d2d can be found [here](https://pr-docs.tlcpack.ai/PR-12496/46/docs/index.html).<!--bot-comment-docs-end-->
   
   <sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] michalpiszczek commented on a diff in pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
michalpiszczek commented on code in PR #12496:
URL: https://github.com/apache/tvm/pull/12496#discussion_r1023292517


##########
python/tvm/script/__init__.py:
##########
@@ -14,8 +14,5 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-"""TVM Script APIs of TVM Python Package, aimed to support TIR"""
-
-from . import tir
-
-from .parser import ir_module, from_source
+"""TVM Script APIs of TVM Python Package"""
+from .parser import ir, ir_module, parse as from_source, tir

Review Comment:
   @junrushao it appears this line, combined with the movement of the old parser from `parser/` to `/parser_v1` means that this PR implicitly switches things over to using the new parser. Am I interpreting this correctly, and was that intentional? 



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] MasterJH5574 commented on a diff in pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
MasterJH5574 commented on code in PR #12496:
URL: https://github.com/apache/tvm/pull/12496#discussion_r1020544241


##########
python/tvm/tir/buffer.py:
##########
@@ -187,13 +187,11 @@ def __getitem__(self, indices):
         if any(isinstance(index, slice) and index.step is None for index in indices):
             region = []
             analyzer = Analyzer()
-            for index in indices:
+            for i, index in zip(range(len(indices)), indices):

Review Comment:
   What about using the following
   ```suggestion
               for i, index in enumerate(indices):
   ```



##########
python/tvm/script/__init__.py:
##########
@@ -14,8 +14,5 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-"""TVM Script APIs of TVM Python Package, aimed to support TIR"""
-
-from . import tir
-
-from .parser import ir_module, from_source
+"""TVM Script APIs of TVM Python Package"""
+from .parser import ir, ir_module, parse as from_source, tir

Review Comment:
   I'm curious about this import. So `ir` becomes `from_source` and `ir_module` becomes `tir`? It doesn't make a lot sense to me, especially because we import three names as two names.



##########
tests/python/unittest/test_tir_transform_lower_cross_thread_reduction.py:
##########
@@ -1329,20 +1247,6 @@ def test_single_reduction_loop_with_tensorize():
     )
 
 
-def test_nested_reduction_loop_with_inner_match_buffers():
-    _check(
-        nested_reduction_loop_with_inner_match_buffers,
-        nested_reduction_loop_with_inner_match_buffers,
-    )
-
-
-def test_nested_reduction_loop_with_outer_match_buffers():
-    _check(
-        nested_reduction_loop_with_outer_match_buffers,
-        nested_reduction_loop_with_outer_match_buffers,
-    )
-
-

Review Comment:
   Hmmm what's the issue with these two tests?



##########
python/tvm/tir/buffer.py:
##########
@@ -187,13 +187,11 @@ def __getitem__(self, indices):
         if any(isinstance(index, slice) and index.step is None for index in indices):
             region = []
             analyzer = Analyzer()
-            for index in indices:
+            for i, index in zip(range(len(indices)), indices):
                 if isinstance(index, slice):
-                    region.append(
-                        Range.from_min_extent(
-                            index.start, analyzer.simplify(index.stop - index.start)
-                        )
-                    )
+                    start = 0 if index.start is None else index.start
+                    stop = self.shape[i] if index.stop is None else index.stop
+                    region.append(Range.from_min_extent(start, analyzer.simplify(stop - start)))

Review Comment:
   Probably we should check if `index.step` is 1 here to ensure safety.



##########
python/tvm/tir/buffer.py:
##########
@@ -202,13 +200,14 @@ def __getitem__(self, indices):
             expr_indices = []
             for index in indices:
                 if isinstance(index, slice):
-                    lanes = analyzer.simplify(
-                        (index.stop - index.start + index.step - 1) // index.step
-                    )
+                    start = 0 if index.start is None else index.start
+                    stop = self.shape[i] if index.stop is None else index.stop
+                    step = 1 if index.step is None else index.step

Review Comment:
   Ditto 🤔 



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] junrushao commented on a diff in pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #12496:
URL: https://github.com/apache/tvm/pull/12496#discussion_r1020621483


##########
python/tvm/tir/buffer.py:
##########
@@ -202,13 +200,14 @@ def __getitem__(self, indices):
             expr_indices = []
             for index in indices:
                 if isinstance(index, slice):
-                    lanes = analyzer.simplify(
-                        (index.stop - index.start + index.step - 1) // index.step
-                    )
+                    start = 0 if index.start is None else index.start
+                    stop = self.shape[i] if index.stop is None else index.stop
+                    step = 1 if index.step is None else index.step

Review Comment:
   we do allow non-trivial step 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.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] junrushao commented on a diff in pull request #12496: [TVMScript] Reorganize the folder structure

Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #12496:
URL: https://github.com/apache/tvm/pull/12496#discussion_r1020597851


##########
tests/python/unittest/test_tir_transform_lower_cross_thread_reduction.py:
##########
@@ -1329,20 +1247,6 @@ def test_single_reduction_loop_with_tensorize():
     )
 
 
-def test_nested_reduction_loop_with_inner_match_buffers():
-    _check(
-        nested_reduction_loop_with_inner_match_buffers,
-        nested_reduction_loop_with_inner_match_buffers,
-    )
-
-
-def test_nested_reduction_loop_with_outer_match_buffers():
-    _check(
-        nested_reduction_loop_with_outer_match_buffers,
-        nested_reduction_loop_with_outer_match_buffers,
-    )
-
-

Review Comment:
   This TVMScript is syntactically wrong. I've reached out to the original author for a fix



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org