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

[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