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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D148414#4290787 <https://reviews.llvm.org/D148414#4290787>, @nikic wrote:

> In D148414#4290762 <https://reviews.llvm.org/D148414#4290762>, @goldstein.w.n wrote:
>
>>> 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.
>>
>> Where do we do the canonicalization?
>
> In foldSelectIntoOp(), with the current constant restriction at https://github.com/llvm/llvm-project/blob/8d163e5045073a5ac570225cc8e14cc9f6d72f09/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp#L507.

Thanks. So how does the following sound:

1. Patch `foldSelectICmpAnd` ->`FoldSelectICmpPow2OrZero` so we can handle:

  define i8 @should_be_doable(i8 %x) {
    %xm1 = sub i8 %x, 1
    %xa = and i8 %xm1, %x
    %cmp = icmp eq i8 %xa, 16
    %s = select i1 %cmp, i8 64, i8 0
    ret i8 %s
  }



2. Update `foldSelectIntoOp` to do constants iff `isPow2(abs(TC - FC))` and `Cond` is  either `(ICmp eq Pow2OrZero, C_Pow2)` or a SignTest

At that point this function should be mostly redundant. There may be some edgecases with `NeutralC` is non-zero (i.e `mul`) which
we will still miss (currently as well) but I'm going to try and remove this function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148414