[PATCH] D142292: [SCEV] Introduce `SCEVSelectExpr`

Sun Jan 22 04:57:14 PST 2023

lebedev.ri added a comment.

In D142292#4071665 <https://reviews.llvm.org/D142292#4071665>, @nikic wrote:

> What I didn't really understand are your examples in the patch description. The referenced godbolt examples don't contain select instructions, and the input program also doesn't look like it should involve selects, so I'm a bit unclear on the relation these examples have to the patch.

I think non-refactoring patches that say what they do, but don't say why they do what they do, should be automatically reverted.
That TLDR is my general motivation. Sure, they no longer have `select`'s because they got canonicalized into min/max, but that isn't tue for everything.

> Looking at the test diffs, I'm wondering whether the canonical form for logical and should be `%a umin_seq %b` or `select %a, %b, false`. I can see arguments in favor of both (the former has symmetry with bitwise and `%a umin %b`, the latter is more in line with IR and extends to logical or, where we don't have `umax_seq`). Probably doesn't matter much either way (I don't think we particularly care about modelling of i1 values in SCEV).

For everything boolean, `select` should be the canonical form.

In D142292#4071669 <https://reviews.llvm.org/D142292#4071669>, @nikic wrote:

> This patch is missing an update of SCEVPoisonCollector, because poison is not propagated from all select operands. (Adding a test for this would be good.)

Sigh.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142292