[Unity][Transform] Implement RemoveUnusedOutputs #16117
Conversation
|
|
||
| int num_outputs_used = 0; | ||
| for (bool used : usage_mask) { | ||
| num_outputs_used += used; |
There was a problem hiding this comment.
I'm surprised you don't get a warning for this cast.
There was a problem hiding this comment.
IIRC, the usual automatic integer-conversion warnings occur from -Wnarrowing, which checks if there's a conversion between integer types that throws away information. Since every value that is allowed in bool can be uniquely written in int, there is no loss of information.
slyubomirsky
left a comment
slyubomirsky
left a comment
There was a problem hiding this comment.
These changes seem to be sound. There are quite a few components but it's clear how everything fits together.
| } | ||
| } | ||
|
|
||
| Expr UnwrapBindings(Expr expr) const { |
There was a problem hiding this comment.
Maybe we should try to generalize some version of this logic? Similar code appears in the FNormalize PR.
There was a problem hiding this comment.
Good point. I think I should make a follow-up PR with all the utility functions from #15916. The ExprVisitor::UnwrapBindings utility would be good to have.
Thank you. My goal was to make it easier to test the parts in isolation, and I'm glad that worked out. I think it's also nice how each of the utility transforms can be beneficial in a general lowering pipeline, while still building together to handle the use case of 'partial tuple usage'. |
This commit implements `relax.transform.RemoveUnusedOutputs`, an extension of the current `DeadCodeElimination` pass. If an internal callee produces multiple outputs, but only some of those outputs are used, the callee can be rewritten to only produce the outputs that are used. This is intended to allow more aggressive DCE in the callee, as reducing the set of outputs can result in intermediate computations becoming unused.
Lunderberg
force-pushed
the
unity_transform_remove_unused_outputs
branch
from
d8c8560 to
bf903a2
Compare
|
I let this PR sit for a bit longer than intended, and am re-running the CI to make sure there haven't been conflicts introduced in the meantime. |
[Unity] Implement RemoveUnusedOutputs transform This commit implements `relax.transform.RemoveUnusedOutputs`, an extension of the current `DeadCodeElimination` pass. If an internal callee produces multiple outputs, but only some of those outputs are used, the callee can be rewritten to only produce the outputs that are used. This is intended to allow more aggressive DCE in the callee, as reducing the set of outputs can result in intermediate computations becoming unused.
2 participants
This commit implements
relax.transform.RemoveUnusedOutputs, an extension of the currentDeadCodeEliminationpass. If an internal callee produces multiple outputs, but only some of those outputs are used, the callee can be rewritten to only produce the outputs that are used. This is intended to allow more aggressive DCE in the callee, as reducing the set of outputs can result in intermediate computations becoming unused.