You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "slyubomirsky (via GitHub)" <gi...@apache.org> on 2023/07/03 18:38:11 UTC

[GitHub] [tvm] slyubomirsky opened a new pull request, #15214: [TIR][UX] Implement privacy annotations in TIR

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

   Implements proposal #14899 for TIR. Adapted from Unity PR #15171, by the same method as PR #15140. Namely, a TIR PrimFunc can be specified to be private (without a global symbol attribute) in TVMScript in the `prim_func` decorator. By default, `PrimFunc`s are not private, so they will have a `global_symbol` attribute that is mapped to their name.
   
   Example usage:
   ```python
   # not private: its global symbol will be "func"
   @T.prim_func
   def func(...): ...
   
   # no global symbol included
   @T.prim_func(private=True)
   def func(...): ...
   ```
   
   This did require changing very, very many tests, unfortunately.


-- 
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] slyubomirsky commented on a diff in pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "slyubomirsky (via GitHub)" <gi...@apache.org>.
slyubomirsky commented on code in PR #15214:
URL: https://github.com/apache/tvm/pull/15214#discussion_r1267286563


##########
python/tvm/tir/schedule/testing.py:
##########
@@ -16,14 +16,33 @@
 # under the License.
 # pylint: disable=dangerous-default-value
 """Testing utilities for the TensorIR schedule API"""
-from typing import Sequence, Union
+from typing import Any, Sequence, Union
 
 import tvm
 from tvm.ir import IRModule, assert_structural_equal
 from tvm.tir import PrimFunc
 from tvm.tir.schedule import Schedule, Trace
 
 
+def assert_structural_equal_gs(

Review Comment:
   I'm not a fan of the abbreviation, but it will probably break a lot of formatting haha. I'll see what will happen



-- 
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] slyubomirsky commented on pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "slyubomirsky (via GitHub)" <gi...@apache.org>.
slyubomirsky commented on PR #15214:
URL: https://github.com/apache/tvm/pull/15214#issuecomment-1642344766

   @tvm-bot rerun


-- 
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] slyubomirsky commented on pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "slyubomirsky (via GitHub)" <gi...@apache.org>.
slyubomirsky commented on PR #15214:
URL: https://github.com/apache/tvm/pull/15214#issuecomment-1636532544

   The test errors turned out to global symbols not matching within the meta scheduler (thanks @vinx13 for the help). Hopefully will be solved shortly.


-- 
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] slyubomirsky commented on pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "slyubomirsky (via GitHub)" <gi...@apache.org>.
slyubomirsky commented on PR #15214:
URL: https://github.com/apache/tvm/pull/15214#issuecomment-1640563701

   @tqchen @vinx13 @junrushao Now that the tests pass, I would appreciate close review, as there were very many tests changed and there seemed to be a lot of functionality affected by global symbols.


-- 
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 #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #15214:
URL: https://github.com/apache/tvm/pull/15214#issuecomment-1618991197

   <!---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-ccs-start-->
    * cc @Hzfengsy, @junrushao, @quic-sanirudh, @shingjan <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-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] junrushao commented on a diff in pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao commented on code in PR #15214:
URL: https://github.com/apache/tvm/pull/15214#discussion_r1267211246


##########
src/ir/function.cc:
##########
@@ -44,4 +45,16 @@ TVM_REGISTER_GLOBAL("ir.BaseFuncWithAttr")
       LOG(FATAL) << "Do not support function type " << func->GetTypeKey();
     });
 
+TVM_REGISTER_GLOBAL("ir.BaseFuncWithoutAttr")

Review Comment:
   Nice! Looks like we always forgot to upstream this piece from the unity branch 



-- 
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] slyubomirsky commented on a diff in pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "slyubomirsky (via GitHub)" <gi...@apache.org>.
slyubomirsky commented on code in PR #15214:
URL: https://github.com/apache/tvm/pull/15214#discussion_r1267290233


##########
python/tvm/tir/schedule/testing.py:
##########
@@ -16,14 +16,33 @@
 # under the License.
 # pylint: disable=dangerous-default-value
 """Testing utilities for the TensorIR schedule API"""
-from typing import Sequence, Union
+from typing import Any, Sequence, Union
 
 import tvm
 from tvm.ir import IRModule, assert_structural_equal
 from tvm.tir import PrimFunc
 from tvm.tir.schedule import Schedule, Trace
 
 
+def assert_structural_equal_gs(

Review Comment:
   It was actually a very easy change via VSCode's "rename" shortcut and applying `black` to all files affected (only 29). :+1:



-- 
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 #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao commented on code in PR #15214:
URL: https://github.com/apache/tvm/pull/15214#discussion_r1267141385


##########
include/tvm/script/ir_builder/tir/ir.h:
##########
@@ -56,7 +56,7 @@ Buffer BufferDecl(Array<PrimExpr> shape, DataType dtype, String buffer_name, Opt
  * \brief The primitive function statement.
  * \return The PrimFuncFrame.
  */
-PrimFuncFrame PrimFunc();
+PrimFuncFrame PrimFunc(const Bool& is_private);

Review Comment:
   let's use `bool` here given we don't need nullability:
   
   ```suggestion
   PrimFuncFrame PrimFunc(bool is_private);
   ```



-- 
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 #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao commented on code in PR #15214:
URL: https://github.com/apache/tvm/pull/15214#discussion_r1267226949


##########
python/tvm/tir/schedule/testing.py:
##########
@@ -16,14 +16,33 @@
 # under the License.
 # pylint: disable=dangerous-default-value
 """Testing utilities for the TensorIR schedule API"""
-from typing import Sequence, Union
+from typing import Any, Sequence, Union
 
 import tvm
 from tvm.ir import IRModule, assert_structural_equal
 from tvm.tir import PrimFunc
 from tvm.tir.schedule import Schedule, Trace
 
 
+def assert_structural_equal_gs(

Review Comment:
   shall we expand `gs` to `global_symbol`? feel free to push back if it's a huge ask (like affecting 80 files)



-- 
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] slyubomirsky commented on a diff in pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "slyubomirsky (via GitHub)" <gi...@apache.org>.
slyubomirsky commented on code in PR #15214:
URL: https://github.com/apache/tvm/pull/15214#discussion_r1267284887


##########
src/ir/function.cc:
##########
@@ -44,4 +45,16 @@ TVM_REGISTER_GLOBAL("ir.BaseFuncWithAttr")
       LOG(FATAL) << "Do not support function type " << func->GetTypeKey();
     });
 
+TVM_REGISTER_GLOBAL("ir.BaseFuncWithoutAttr")

Review Comment:
   Yeah lol



-- 
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] slyubomirsky commented on pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "slyubomirsky (via GitHub)" <gi...@apache.org>.
slyubomirsky commented on PR #15214:
URL: https://github.com/apache/tvm/pull/15214#issuecomment-1624473474

   @junrushao Any insight on the test failures involving `check_sketches` (e.g., in `tests/python/unittest/test_meta_schedule_schedule_rule_cross_thread_reduction.py`)? Getting rid of the global symbols in those tests didn't affect them, so I'm not sure where my changes could affect these


-- 
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] slyubomirsky commented on a diff in pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "slyubomirsky (via GitHub)" <gi...@apache.org>.
slyubomirsky commented on code in PR #15214:
URL: https://github.com/apache/tvm/pull/15214#discussion_r1267284608


##########
include/tvm/script/ir_builder/tir/ir.h:
##########
@@ -56,7 +56,7 @@ Buffer BufferDecl(Array<PrimExpr> shape, DataType dtype, String buffer_name, Opt
  * \brief The primitive function statement.
  * \return The PrimFuncFrame.
  */
-PrimFuncFrame PrimFunc();
+PrimFuncFrame PrimFunc(const Bool& is_private);

Review Comment:
   Thanks, I've done it



-- 
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 merged pull request #15214: [TIR][UX] Implement privacy annotations in TIR

Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao merged PR #15214:
URL: https://github.com/apache/tvm/pull/15214


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