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

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 7 13:52:48 PDT 2018


spatel updated this revision to Diff 145523.
spatel marked 8 inline comments as done.
spatel added a comment.

Patch updated:
I think what made this patch confusing and overly complex is trying to recycle existing instructions (swapping operands rather than just creating new instructions).

As I mentioned in an earlier comment, I don't see a good reason to do that, so let's do the easy thing: use IRBuilder to create new instructions. This has 3 benefits and shrinks the patch:

1. It simplifies the code needed for commutative canonicalization (looks more like the original instcombine patch now).
2. It means we don't need to manually update debug info (IRBuilder does the right thing automatically).
3. It makes the IR flag clearing/propagation cleaner.


https://reviews.llvm.org/D45842

Files:
  include/llvm/Transforms/Scalar/Reassociate.h
  lib/Transforms/Scalar/Reassociate.cpp
  test/Transforms/Reassociate/matching-binops.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45842.145523.patch
Type: text/x-patch
Size: 12938 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180507/922c0caf/attachment.bin>


More information about the llvm-commits mailing list