[PATCH] D45842: [Reassociate] swap binop operands to increase factoring potential

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In https://reviews.llvm.org/D45842#1081703, @spatel wrote:

> In https://reviews.llvm.org/D45842#1081077, @bjope wrote:
>
> > As far as I can see this is another case of reusing existing Instructions/Values, while changing the actual value that the Instruction produce, right?
> >
> > Take a look at the debug-info fixes I made here for a similar problem in RewriteExprTree: https://reviews.llvm.org/D45975
> >  I suspect that you may need to discard debug-info in a similar way as in https://reviews.llvm.org/D45975, somewhere inside your new function swapOperandsToMatchBinops.
>
>
> Thanks! You're correct that we're recycling instructions here (not sure why we don't just create new instructions with a Builder?). The funny thing about all the examples here is that after we swap operands in swapOperandsToMatchBinops(), we end up going through the main reassociation loop again and make more changes which triggers your code https://reviews.llvm.org/D45975, so I already see 'undef' in the right places.
>
> Nevertheless, it's a small change to extract and call that code, so let me do that to be safe.


Great! Your last update seems to fix my concern about debug-info. I'll let someone else review the actual code transformation done here.


https://reviews.llvm.org/D45842