[PATCH] D148414: [InstCombine] Expand `foldSelectICmpAndOr` -> `foldSelectICmpAndBinOp` to work for any binop
Noah Goldstein via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 27 14:50:51 PDT 2023
goldstein.w.n added a comment.
In D148414#4290296 <https://reviews.llvm.org/D148414#4290296>, @nikic wrote:
> 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.
Fixed, have proofs for all the binops I added.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148414/new/
https://reviews.llvm.org/D148414
More information about the llvm-commits
mailing list