[PATCH] D148414: [InstCombine] Expand `foldSelectICmpAndOr` -> `foldSelectICmpAndBinOp` to work for any binop

[Nikita Popov via Phabricator via llvm-commits]

nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.

Why is this legal for all binops? Doesn't this transform require that the neutral element of the binop is zero? So it would work for or, xor or add, but not for mul or and for example?

I think this entire transform should probably be handled as a two step process: First, `Cond ? X : BinOp(X, C)` should become `BinOp(X, Cond ? NeutralC : C)` and then this fold should work on `Cond ? 0 : C` as the root. We actually already do the former canonicalization, but not in the case where `C` is constant. This will cleanly separate out the actual binop handling. Only disadvantage is that we won't be able to handle the case where the binop has multiple uses anymore, but that's a general issue with composable folds that we can ignore unless there is reason to believe that multi-use is motivating for the current handling.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148414/new/

https://reviews.llvm.org/D148414