Skip to content

[Unity][Pass] Reuse prior infra to implement more complete DCE#14334

Merged
MasterJH5574 merged 6 commits intoapache:unityfrom
ganler:dce
Mar 20, 2023
Merged

[Unity][Pass] Reuse prior infra to implement more complete DCE#14334
MasterJH5574 merged 6 commits intoapache:unityfrom
ganler:dce

Conversation

@ganler
Copy link
Contributor

@ganler ganler commented Mar 19, 2023

As a follow-up to #14262, I just noticed that I previously implemented a function called RemoveAllUnused (#14043) that can do function-wise DCE and should be more complete than #14262 as RemoveAllUnused can also remove dead dataflow blocks (added two test-cases to show that).

Sorry, I should have mentioned that earlier to avoid duplication... but still I am putting a patch here in case it is desired.

cc: @spectrometerHBH @tqchen

@tvm-bot
Copy link
Collaborator

tvm-bot commented Mar 19, 2023

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 by @-ing them in a comment.

Generated by tvm-bot

@ganler
Copy link
Contributor Author

ganler commented Mar 19, 2023

At the high level, #14043 is based on a fix-point algorithm to remove unused dataflow statements iteratively. While I think @spectrometerHBH 's impl is more efficient based on the assumption from the dataflow block.

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ganler
Copy link
Contributor Author

ganler commented Mar 19, 2023

BTW, although this patch uses RemoveAllUnused over @spectrometerHBH 's implementation, it did not remove the prior implementation. Not sure if we should just keep it in case of future usage or just trim it to simplify the code... It would be great if @spectrometerHBH can help decide. :)

@tqchen
Copy link
Member

tqchen commented Mar 19, 2023

I think we can simply trim it and consolidate the use

@ganler
Copy link
Contributor Author

ganler commented Mar 19, 2023

For now:

  • tvm.relax.transform.DeadCodeElimination is the pass for running DCE over an IRModule.
  • tvm.relax.analysis.remove_all_unused is a function for running DCE over a function.

tvm.relax.transform.DeadCodeElimination is implemented based on tvm.relax.analysis.remove_all_unused. I did not sync the function name for the two, as I saw there are other uses for RemoveAllUnused so just want to be conservative first.

@MasterJH5574
Copy link
Contributor

@ganler Thanks for the clarification!

@MasterJH5574 MasterJH5574 merged commit 873ab3d into apache:unity Mar 20, 2023
tqchen pushed a commit that referenced this pull request Mar 20, 2023
As a follow-up to #14262, I just noticed that I previously implemented a function called `RemoveAllUnused` (#14043) that can do function-wise DCE and should be more complete than #14262 as `RemoveAllUnused` can also remove dead dataflow blocks (added two test-cases to show that). 

For now:
- `tvm.relax.transform.DeadCodeElimination` is the pass for running DCE over an IRModule.
- `tvm.relax.analysis.remove_all_unused` is a function for running DCE over a function.

`tvm.relax.transform.DeadCodeElimination` is implemented based on `tvm.relax.analysis.remove_all_unused`. I did not sync the function name for the two, as I saw there are other uses for `RemoveAllUnused` so just want to be conservative first.
tqchen pushed a commit that referenced this pull request Apr 1, 2023
As a follow-up to #14262, I just noticed that I previously implemented a function called `RemoveAllUnused` (#14043) that can do function-wise DCE and should be more complete than #14262 as `RemoveAllUnused` can also remove dead dataflow blocks (added two test-cases to show that). 

For now:
- `tvm.relax.transform.DeadCodeElimination` is the pass for running DCE over an IRModule.
- `tvm.relax.analysis.remove_all_unused` is a function for running DCE over a function.

`tvm.relax.transform.DeadCodeElimination` is implemented based on `tvm.relax.analysis.remove_all_unused`. I did not sync the function name for the two, as I saw there are other uses for `RemoveAllUnused` so just want to be conservative first.
tqchen pushed a commit that referenced this pull request Apr 1, 2023
As a follow-up to #14262, I just noticed that I previously implemented a function called `RemoveAllUnused` (#14043) that can do function-wise DCE and should be more complete than #14262 as `RemoveAllUnused` can also remove dead dataflow blocks (added two test-cases to show that). 

For now:
- `tvm.relax.transform.DeadCodeElimination` is the pass for running DCE over an IRModule.
- `tvm.relax.analysis.remove_all_unused` is a function for running DCE over a function.

`tvm.relax.transform.DeadCodeElimination` is implemented based on `tvm.relax.analysis.remove_all_unused`. I did not sync the function name for the two, as I saw there are other uses for `RemoveAllUnused` so just want to be conservative first.
tqchen pushed a commit that referenced this pull request Apr 1, 2023
As a follow-up to #14262, I just noticed that I previously implemented a function called `RemoveAllUnused` (#14043) that can do function-wise DCE and should be more complete than #14262 as `RemoveAllUnused` can also remove dead dataflow blocks (added two test-cases to show that). 

For now:
- `tvm.relax.transform.DeadCodeElimination` is the pass for running DCE over an IRModule.
- `tvm.relax.analysis.remove_all_unused` is a function for running DCE over a function.

`tvm.relax.transform.DeadCodeElimination` is implemented based on `tvm.relax.analysis.remove_all_unused`. I did not sync the function name for the two, as I saw there are other uses for `RemoveAllUnused` so just want to be conservative first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants