[PATCH] D41574: [Transforms] Adding a WeakReassociate pass

[Omer Paparo Bivas via Phabricator via llvm-commits]

opaparo added a comment.

In https://reviews.llvm.org/D41574#964073, @davide wrote:

> It's not entirely clear to me why you need another reassociate pass.
>  We should really try to share as much code as possible with the existing one instead of adding maintenance burden.


Although both of the passes perform reassociation, they are essentially very different.
The Reassociate pass aggressively modifies the topography of the expression tree, basically rewriting it, while the WeakReassociate pass uses the existing topography.
The Reassociate pass creates new instructions while the WeakReassociate pass does not.
Moreover, their method of reassociation is completely different. To modify the existing Reassociate pass to supports reassociation of the form (a_0 + a_1) + (a_2 + a_3) -> (a_0 + a_2) + (a_1 + a_3) will take a great amount of changes, changes that will almost certainly collide with existing behavior and test cases. For example, how will it decide whether (a_0 + a_1) + (a_2 + a_3) will be transformed into a_0 + (a_1 + (a_2 + a_3)) like its current behavior or to (a_0 + a_2) + (a_1 + a_3) like the new behavior?
The existing Reassociate pass lacks, by design, several kinds of reassociations, kinds that I believe can find a home in the new pass.
IMO, maintaining one big pass that has two distinguished purposes can prove to be a greater burden than maintaining two smaller passes, each dedicated to its own purpose.


Repository:
  rL LLVM

https://reviews.llvm.org/D41574