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

[GitHub] [tvm] Lunderberg opened a new pull request, #15507: [Unity] Remove unused local function definitions

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

   Prior to this commit, the `relax.analysis.remove_all_unused` utility only removed variable bindings that occur within a dataflow block. This satisfies that the condition that a variable can be removed if is unused and if its computation has no side effects.  However, it doesn't include any values from a `BindingBlock`.
   
   This commit updates the `RemoveUnusedVars` pass to also inspect `BindingBlock` instances for removeable variables.  Currently, this is limited to unused local function definitions.


-- 
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] kparzysz-quic merged pull request #15507: [Unity] Remove unused local function definitions

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


-- 
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] Lunderberg commented on a diff in pull request #15507: [Unity] Remove unused local function definitions

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


##########
tests/python/relax/test_analysis.py:
##########
@@ -117,6 +117,53 @@ def main(x: R.Tensor((32, 32), "float32")) -> R.Tensor:
     tvm.ir.assert_structural_equal(optimized, GroundTruth["main"])
 
 
+def test_binding_block_remove_all_unused_without_dataflow():
+    @tvm.script.ir_module
+    class IdentityUnused:
+        @R.function
+        def main(x: R.Tensor((32, 32), "float32")) -> R.Tensor:
+            lv0 = x
+            unused0 = R.call_dps_packed("my_sigmoid", (x,), R.Tensor((32, 32), dtype="float32"))
+            unused1 = R.call_dps_packed(
+                "my_dps_func", (unused0,), R.Tensor((32, 32), dtype="float32")
+            )
+            z = R.call_packed("vm.builtin.copy", lv0, sinfo_args=(R.Tensor((32, 32), "float32")))
+            return z
+
+    optimized = remove_all_unused(IdentityUnused["main"])
+
+    GroundTruth = IdentityUnused

Review Comment:
   For this specific change, because (to my understanding), the `call_dps_packed` doesn't guarantee that it has no side effects.  Within a `R.dataflow()` environment, the expression may not have side effects, so it is safe to remove.
   
   For newly-unused objects, I believe that is handled by the `GetUnusedVars` function, called during construction.  It iteratively walks from the function outputs to the predecessors required to generate those outputs, finding the set of all variables required to generate the output.  Anything not within that set is considered unused and can be removed, even it occurs within another expression (because that expression is also going to be removed).



-- 
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] kparzysz-quic commented on a diff in pull request #15507: [Unity] Remove unused local function definitions

Posted by "kparzysz-quic (via GitHub)" <gi...@apache.org>.
kparzysz-quic commented on code in PR #15507:
URL: https://github.com/apache/tvm/pull/15507#discussion_r1289020064


##########
tests/python/relax/test_analysis.py:
##########
@@ -117,6 +117,53 @@ def main(x: R.Tensor((32, 32), "float32")) -> R.Tensor:
     tvm.ir.assert_structural_equal(optimized, GroundTruth["main"])
 
 
+def test_binding_block_remove_all_unused_without_dataflow():
+    @tvm.script.ir_module
+    class IdentityUnused:
+        @R.function
+        def main(x: R.Tensor((32, 32), "float32")) -> R.Tensor:
+            lv0 = x
+            unused0 = R.call_dps_packed("my_sigmoid", (x,), R.Tensor((32, 32), dtype="float32"))
+            unused1 = R.call_dps_packed(
+                "my_dps_func", (unused0,), R.Tensor((32, 32), dtype="float32")
+            )
+            z = R.call_packed("vm.builtin.copy", lv0, sinfo_args=(R.Tensor((32, 32), "float32")))
+            return z
+
+    optimized = remove_all_unused(IdentityUnused["main"])
+
+    GroundTruth = IdentityUnused

Review Comment:
   Shouldn't the two `unused`s be removed?  I think this whole algorithm (not your change specifically) is somewhat deficient if it doesn't keep track of newly-unused objects.



-- 
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 #15507: [Unity] Remove unused local function definitions

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

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