[Relax] Improve CanonicalizeBindings in DataflowVar edge case #16783
Conversation
If there is a trivial binding of `Var = DataflowVar`, but the non-dataflow variable is never used outside the dataflow block in which is is declared, then we should keep the name of the upstream `DataflowVar`, as it is more likely to be the human-readable name (e.g. a function parameter).
slyubomirsky
left a comment
slyubomirsky
left a comment
There was a problem hiding this comment.
This is a nice improvement, though there's a significant typo that should be fixed before we merge 😅 Thank you!
| // Case 3: DataflowVar = DataflowVar | ||
| // Case 4a: Var = DataflowVar, but used outside this DataflowBlock | ||
| // | ||
| // For these three cases, the trivial binding can be |
There was a problem hiding this comment.
| // For these three cases, the trivial binding can be | |
| // For these four cases, the trivial binding can be |
😉
There was a problem hiding this comment.
Off by one errors, my two one nemesis! (Thank you, and fixed.)
| // Case 1: Var = Var | ||
| // Case 2: DataflowVar = Var | ||
| // Case 3: DataflowVar = DataflowVar | ||
| // Case 4a: Var = DataflowVar, but used outside this DataflowBlock |
There was a problem hiding this comment.
| // Case 4a: Var = DataflowVar, but used outside this DataflowBlock | |
| // Case 4a: Var = DataflowVar, but not used outside this DataflowBlock |
It appears the text was copied from case 4b. I also think it would be clearer to write out that it is the var that is not used outside the DF block
There was a problem hiding this comment.
Thank you, and updated the comment to be more explicit. I've also changed "this DataflowBlock" to "the DataflowBlock containing the binding", since this function is called after the entire function is visited, not during the visit of any specific dataflow block.
slyubomirsky
left a comment
slyubomirsky
left a comment
There was a problem hiding this comment.
Thanks for the correction.
Lunderberg
deleted the
relax_improve_canonicalize_bindings_dataflow_handling
branch
…#16783) * [Relax] Improve CanonicalizeBindings in DataflowVar edge case If there is a trivial binding of `Var = DataflowVar`, but the non-dataflow variable is never used outside the dataflow block in which is is declared, then we should keep the name of the upstream `DataflowVar`, as it is more likely to be the human-readable name (e.g. a function parameter). * Update comment for used/not used Var * ci bump
2 participants
If there is a trivial binding of
Var = DataflowVar, but the non-dataflow variable is never used outside the dataflow block in which is is declared, then we should keep the name of the upstreamDataflowVar, as it is more likely to be the human-readable name (e.g. a function parameter).