[PATCH] D153963: [InstCombine] Fold binop of select and cast of select condition

Thu Jul 13 16:12:18 PDT 2023

goldstein.w.n added a comment.

In D153963#4498090 <https://reviews.llvm.org/D153963#4498090>, @antoniofrighetto wrote:

> @goldstein.w.n, added multiuse select and non-const sides tests (last three tests), proofs updated (https://alive2.llvm.org/ce/z/m_vGWd). Tests are on a dedicated commit.
>
> It seems RHS ext & LHS sel operands are canonicalized as such in `SimplifyAssociativeOrCommutative` for `add`/`mul`.

Okay thats fine (also other binops).

> When the binop is a subtraction, being non-commutative, the semantic of the operation will change if the operands are swapped. I tried extending the helper to support swapped operands too (LHS ext & RHS sel), yet, the transformation we obtain appears to be correct only when the `select` has both const sides. With non-const sides, the transformation fails (proof: https://alive2.llvm.org/ce/z/BZU9cR, 2 transformations failed out of 4). I don't believe we should introduce specialization in the helper for const-only subs.

non-const is fine AFAICT, youre proof is just a little buggy: https://alive2.llvm.org/ce/z/4yWy3R
Its okay to skip for now, but please add TODO for sub.

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

https://reviews.llvm.org/D153963