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/18 01:42:12 UTC

[GitHub] [tvm] slyubomirsky opened a new pull request, #15341: [Unity][Transform] Elide redundant bindings of dataflow vars

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

   This PR addresses the second issue in #15245. In particular, it handles the case that can arise when using `CanonicalizeBindings` of having a dataflow var whose only use is being bound to the output var, like so:
   
   ```python
   @R.function
   def main() -> R.Tensor((), "int32"):
           with R.dataflow():
               y = R.const(1)
               n = y
               R.output(n)
           return n
   ```
   The only use for `y` is to be bound to `n`, which is the `DataflowBlock`'s output.
   
   This PR introduces a check in `DeadCodeElimination` that detects this case and eliminates the intermediate binding, leaving in this example simply this:
   ```python
   @R.function
   def main() -> R.Tensor((), "int32"):
           with R.dataflow():
               n = R.const(1)
               R.output(n)
           return n
   ```


-- 
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 #15341: [Unity][Transform] Elide redundant bindings of dataflow vars

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

   <!---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 @quic-sanirudh <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] slyubomirsky commented on pull request #15341: [Unity][Transform] Elide redundant bindings of dataflow vars

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

   Thank you for the pointer about the pattern-matching language, I think that would probably indeed be a better way to implement this. As for where to include the functionality, what I think I will make this a separate pass; we could instead combine `CanonicalizeBindings`, `DeadCodeElimination`, this pass, and maybe some others into a single larger simplification pass (that could run until fixpoint).


-- 
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] sunggg commented on a diff in pull request #15341: [Unity][Transform] Elide redundant bindings of dataflow vars

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


##########
tests/python/relax/test_transform_dead_code_elimination.py:
##########
@@ -448,5 +448,79 @@ def main(
     verify(Input, Expected)
 
 
+def test_elide_intermediate_dataflow_var_simple():
+    @tvm.script.ir_module
+    class Input:
+        @R.function
+        def main() -> R.Tensor((), "int32"):
+            with R.dataflow():
+                y = R.const(1)
+                n = y
+                R.output(n)
+            return n
+
+    @tvm.script.ir_module
+    class Expected:
+        @R.function
+        def main() -> R.Tensor((), "int32"):
+            with R.dataflow():
+                n = R.const(1)
+                R.output(n)
+            return n
+
+    verify(Input, Expected)
+
+
+def test_elide_intermediate_dataflow_var_match_cast():

Review Comment:
   This seems like constant folding.



-- 
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 #15341: [Unity][Transform] Elide redundant bindings of dataflow vars

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

   I'm a little surprised by how verbose this check turned out to be, so I wonder if there's a more elegant way to implement it that I've managed to miss. Perhaps the condition is too complex and there are more circumstances where we might want to eliminate these intermediate bindings.
   
   I'm also not sure if the dead code elimination pass is the best place to make this happen, though I don't know what other pass would be better for this check. It could potentially be its own mini-pass.


-- 
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 #15341: [Unity][Transform] Elide redundant bindings of dataflow vars

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

   Unfortunately, it seems the pattern given in @sunggg's example will not handle the case of multiple variables to fold, like this one:
   ```python
   @R.function
   def func() -> R.Tensor((), "int32"):
       with R.dataflow():
           y = R.const(1)
           z = R.const(2)
           m = y
           n = z
           R.output(m, n)
       return n
   ```
   The pattern given will identify `y` and `m`, but not `n` and `z`.


-- 
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 #15341: [Unity][Transform] Elide redundant bindings of dataflow vars

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

   I've separated the logic out to another pass. I will investigate using the pattern language to make the matching more concise, though I think it may not actually suffice here because I would still need to search to find _which_ var to eliminate.


-- 
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] csullivan merged pull request #15341: [Unity][Transform] Elide redundant bindings of dataflow vars

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


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