[PATCH] D21299: [Codegen Prepare] Swap commutative binops before splitting branch condition.

Renato Golin via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 18 09:49:59 PDT 2016


rengolin added a comment.

In http://reviews.llvm.org/D21299#461003, @bmakam wrote:

> In principle, this change is target independent because it reassociates binary operands to simplify branches. The reassociation pass is designed for transformations that will help down the line optimizations such as constant propagation, GCSE, LICM, PRE etc.. so I moved it down to CGP.


The re-association is target independent, but guessing which branch will be taken probably isn't, as it depends on the branch-predictor, which are wildly different on some targets / workloads.

> I can certainly verify for A57 and know for a fact that it improves spec2006/mcf on A57 as well. However, I am uncertain of reliably testing and verifying on other targets.


At least for A57 would be nice.

The new flag should suffice. It can also allow other target maintainers to test on their arches by adding the feature and benchmarking, and then commit the change if profitable. Only then, if this is universally true, we could remove the flag and make it a generic pass.

cheers,
--renato


http://reviews.llvm.org/D21299





More information about the llvm-commits mailing list