[PATCH] D93888: [GVN] If zext X == N or sext X == N, then X == trunc N.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D93888#2483368 <https://reviews.llvm.org/D93888#2483368>, @wecing wrote:

> @nikic Would you mind explaining why? If there is a good reason we cannot do this in NewGVN then D93850 <https://reviews.llvm.org/D93850> might be the best we could do.

Handling it in NewGVN and IPSCCP requires handling it in PredicateInfo, by introducing new SSA variables and performing SSA renaming in relevant branches not just for comparison operands, but also for operands-of-operands. That is, for `br icmp (zext X), C` we currently introduce up to two new variables for `(zext X)` and rename to them in the relevant branch. Handling the pattern here would require introducing a new variable for `X` as well. Of course it's possible to do this,  but, but it leads to a collection of special cases rather than a principled approach. Especially as we have to deal with this in multiple passes, it's better to avoid it if we can.

In D93888#2483911 <https://reviews.llvm.org/D93888#2483911>, @lebedev.ri wrote:

> I'm not yet convinced we can do this in instcombine,
> because why can we do this without a profitability check,
> like the fold just below that one does?

The profitability check is there to avoid creating illegal types that did not exist in the original IR. If we are simply dropping a zext, we're using a type that already existed in the IR, and would generally consider that to be fine. We could be even more conservative and require the zext-ed from type to be legal (doesn't make much diff in practice). Dropping a `zext i32 to i64` to switch over an `i32` is quite different from inserting a trunc to `i3` and switching over `i3`. The former seems like an unambiguous improvement, while the latter will regress more likely than not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93888