[PATCH] D51398: [InstCombine] Fold (min/max ~X, Y) -> ~(max/min X, ~Y) when Y is freely invertible

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D51398#1218774, @craig.topper wrote:

> Address review comments.
>
> Not sure about the metadata copying. I'm not sure if I can rely on LHS and RHS to be in the same order as the original select.


I think that will always be true because LHS/RHS are set based on the original cmp, and we create the new min/max with A/B mapped to LHS/RHS...but some asserts would help (dis)prove it.

Also, repeating Roman's earlier request - we could eliminate the code duplication by making a helper/lambda and reversing the operands in the call?
Something like:

  if (Instruction *Not = moveNotAfterMinMax(LHS, RHS))
    return Not;
  if (Instruction *Not = moveNotAfterMinMax(RHS, LHS))
    return Not;

But I think that wouldn't preserve the A/B mapping mentioned above. If you see a way to resolve that, that would be nice. LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D51398