[GitHub] [tvm] shingjan commented on a change in pull request #10099: [TVMScript] Support T.buffer_decl using data pointer from Let/Allocate

[GitBox <gi...@apache.org>]

shingjan commented on a change in pull request #10099:
URL: https://github.com/apache/tvm/pull/10099#discussion_r798134591



##########
File path: python/tvm/script/parser.py
##########
@@ -1138,6 +1141,29 @@ def transform_TypeTuple(self, node):
         """
         return [self.transform(value) for value in node.values]
 
+    def transform_TypeApply(self, node):
+        """Visitor for Type[Type] expressions.
+
+        Mostly used for ``T.Ptr`` expressions.
+        """
+        func = self.transform(node.func_name)
+
+        if not isinstance(func, ty.TypeGeneric):
+            self.report_error(f"Expected a type but found {type(func).__name__}", node.span)

Review comment:
       `Expect a type` -> `Expect  a GenericPtrType` maybe be better

##########
File path: python/tvm/script/parser.py
##########
@@ -1138,6 +1141,29 @@ def transform_TypeTuple(self, node):
         """
         return [self.transform(value) for value in node.values]
 
+    def transform_TypeApply(self, node):
+        """Visitor for Type[Type] expressions.
+
+        Mostly used for ``T.Ptr`` expressions.
+        """
+        func = self.transform(node.func_name)
+
+        if not isinstance(func, ty.TypeGeneric):
+            self.report_error(f"Expected a type but found {type(func).__name__}", node.span)
+
+        param_types = []

Review comment:
       there is a `transform_TypeTuple` impl that may be useful here

##########
File path: python/tvm/script/tir/ty.py
##########
@@ -65,6 +70,8 @@ class GenericTupleType(TypeGeneric):  # pylint: disable=abstract-method
     """
 
     def __getitem__(self, vtypes):
+        if isinstance(vtypes, TypeGeneric):

Review comment:
       This seems to be a bug fix right?

##########
File path: tests/python/unittest/test_tvmscript_roundtrip.py
##########
@@ -3255,5 +3255,44 @@ def test_root_attr():
     tvm.ir.assert_structural_equal(func, rt_func, True)
 
 
+@T.prim_func
+def func_T_ptr_let_statement(
+    args: T.handle, arg_type_ids_handle: T.Ptr[T.int32], num_args: T.int32
+) -> None:
+    # buffer definition
+    arg_type_ids = T.buffer_decl([2], dtype="int32", data=arg_type_ids_handle)
+    # body
+    assert num_args == 2, "main: num_args should be 3"

Review comment:
       mismatch between the code and comment here. BTW is this check necessary here?

##########
File path: python/tvm/script/parser.py
##########
@@ -1138,6 +1141,29 @@ def transform_TypeTuple(self, node):
         """
         return [self.transform(value) for value in node.values]
 
+    def transform_TypeApply(self, node):
+        """Visitor for Type[Type] expressions.
+
+        Mostly used for ``T.Ptr`` expressions.
+        """
+        func = self.transform(node.func_name)
+
+        if not isinstance(func, ty.TypeGeneric):
+            self.report_error(f"Expected a type but found {type(func).__name__}", node.span)
+
+        param_types = []
+        for param in node.params:
+            param_type = self.transform(param)
+            if not isinstance(param_type, ty.TypeGeneric):

Review comment:
       Does this for-loop imply that all params of `TypeApply` call will be transformed into `GenericPtrType`? If so can we specify that here as well instead of `ty.TypeGeneric` 

##########
File path: python/tvm/script/parser.py
##########
@@ -1138,6 +1141,29 @@ def transform_TypeTuple(self, node):
         """
         return [self.transform(value) for value in node.values]
 
+    def transform_TypeApply(self, node):
+        """Visitor for Type[Type] expressions.
+
+        Mostly used for ``T.Ptr`` expressions.
+        """
+        func = self.transform(node.func_name)
+
+        if not isinstance(func, ty.TypeGeneric):

Review comment:
       Use of `GenericPtrType` here maybe more accurate

##########
File path: python/tvm/script/tir/ty.py
##########
@@ -44,6 +44,9 @@ def __init__(self, vtype):
         self.type = vtype
 
     def evaluate(self):
+        if isinstance(self.type, tvm.ir.Type):

Review comment:
       I thought this change only impact `GenericPtrType` instead of `ConcreteType` here. Is that the case?




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