[PATCH] D116058: [InstCombine] Convert binop(phi, v) to phi(binop) for constant phi operands

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D116058#3233557 <https://reviews.llvm.org/D116058#3233557>, @nikic wrote:

> I think it's pretty clear that `op(phi(c1, c2))` to `phi(op(c1), op(c2))` is not the correct canonicalization for the middle-end optimizer, because it increases redundancy. Materialization of phi operands is firmly a backend matter, and this transform should live somewhere in the backend IR pipeline. I do agree that the general idea makes sense if done right (i.e. not overly aggressive, as SpeculateAroundPhis was).

Agree - I think the first test diff (`shrinkLogicAndPhi1`) is the inverse of what -simplifycfg would try to do, so this is too general. Alternative patches that should solve the motivating bug are proposed in D115914 <https://reviews.llvm.org/D115914> and D117110 <https://reviews.llvm.org/D117110>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116058