[PATCH] D45862: [InstCombine] Remove decanonicalizing transforms of selects

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 27 09:59:21 PDT 2018


spatel added a comment.

In https://reviews.llvm.org/D45862#1080423, @mkazantsev wrote:

> I am confused. Didn't we agree on that replacement of selects with bit-wise logic, if it's profitable, should be a part of backend's work, and here we want selects because they have a more clear semantics that can be used by other transforms, rather than teaching every single transform to recognize the bit magic as select? You are giving some examples where codegen definitely could do what you are showing.




1. I think it's clearly better for IR to eliminate the select if we can reduce the instruction count too. It can lead to more reduction here in instcombine as seen in test15g. So that's why I'm proposing to add optimizations for these patterns: https://rise4fun.com/Alive/XG7 in https://reviews.llvm.org/D46086 (note that test15g is not regressed there).
2. Even if you disagree with having those transforms, I don't think we should remove it as the first step. We're already going to be at risk for revert due to perf regressions with https://reviews.llvm.org/D46086 even though those are all IR improvements based on our reasoning so far. I'm purposely trying to minimize the diffs in that step, so we have a better chance of pushing this through.


https://reviews.llvm.org/D45862





More information about the llvm-commits mailing list