Skip to content

Conversation

@Lunderberg
Copy link
Contributor

Prior to this commit, the relax.transform.CanonicalizeBindings transform would detect trivial bindings var_y = var_x, and replace later usage of var_y with var_x. However, the trivial binding var_y = var_x would be left in the canonicalized function.

This commit updates the CanonicalizeBindings transform to remove trivial bindings. This is not intended as a full dead-code elimination, as that is better handled as a separate pass, but is instead intended to avoid introduction of dead code during canonicalization.

Prior to this commit, the `relax.transform.CanonicalizeBindings`
transform would detect trivial bindings `var_y = var_x`, and replace
later usage of `var_y` with `var_x`.  However, the trivial binding
`var_y = var_x` would be left in the canonicalized function.

This commit updates the `CanonicalizeBindings` transform to remove
trivial bindings.  This is not intended as a full dead-code
elimination, as that is better handled as a separate pass, but is
instead intended to avoid introduction of dead code during
canonicalization.
@Lunderberg Lunderberg force-pushed the unity_avoid_unused_trivial_bindings_in_canonicalization branch from 3c2f562 to 0363f0b Compare September 29, 2023 14:20
@slyubomirsky
Copy link
Contributor

Oh lol I just folded this functionality into #15791 without seeing this. This implementation is slightly more compact so we can use it and I can clean up #15791.

Copy link
Contributor

@slyubomirsky slyubomirsky left a comment

Choose a reason for hiding this comment

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

This all seems sound, thank you for the change!

@slyubomirsky slyubomirsky merged commit 233ac7a into apache:unity Oct 2, 2023
@Lunderberg Lunderberg deleted the unity_avoid_unused_trivial_bindings_in_canonicalization branch October 3, 2023 14:01
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.

2 participants