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

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 19 15:30:24 PDT 2018


spatel created this revision.
spatel added reviewers: escha, opaparo, davide, fhahn, efriedma, qcolombet.
Herald added a subscriber: mcrosier.

If we have a pair of binops feeding another pair of binops, rearrange the operands so the matching pair are together because that allows easy factorization folds to happen in instcombine:
((X << S) & Y) & (Z << S) --> ((X << S) & (Z << S)) & Y (reassociation)

  --> ((X & Z) << S) & Y (factorize shift from 'and' ops optimization)

This is part of solving PR37098:
https://bugs.llvm.org/show_bug.cgi?id=37098

Note that there's an instcombine version of this patch attached there. This reassociate patch took about 10x more effort, so I hope this is the preferred direction. :)

For reasons I still don't completely understand, reassociate does this kind of transform sometimes, but misses everything in my motivating cases.

This patch on its own is gluing an independent cleanup chunk to the end of the existing RewriteExprTree() loop. But if it's approved, I think we can build on it and do something stronger to better order the full expression tree like https://reviews.llvm.org/D40049. That might be an alternative to the proposal to add a separate reassociation pass like https://reviews.llvm.org/D41574.


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.143169.patch
Type: text/x-patch
Size: 11169 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180419/3e1f9365/attachment.bin>


More information about the llvm-commits mailing list