[PATCH] D51964: [InstCombine] Fold (xor (min/max X, Y), -1) -> (max/min ~X, ~Y) when X and Y are freely invertible.

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 13 08:34:44 PDT 2018


spatel added a comment.

I looked a bit closer at what's going on in PR38915. This patch fixes that particular case, but I'm worried that we still have the same bug lurking if we commit this without solving the underlying problem.

I think these combines are fighting because we don't have a way to accurately assess 'one-use' in terms of another min/max within IsFreeToInvert(). This could be the best argument yet for integer min/max intrinsics because we're going to have to keep special-casing / adding those !X->hasNUsesOrMore(3) hacks to account for min/max transforms...or maybe this + whatever else is needed to recommit https://reviews.llvm.org/rL341674 is enough?

The problem might also be solved by adjusting the min/max bailout that we have within visitICmp. We don't optimize cmps that are part of min/max because we're scared to break a min/max.

There's another underlying problem that I'd like to look that I've been putting off: we have a bunch of min/max of min/max transforms in instcombine, but they really belong in instsimplify because we're just deleting ops. It's not a complete solution, but if we fix that, we're less likely to have this problem in instcombine because some patterns would be killed before we could get into trouble.


Repository:
  rL LLVM

https://reviews.llvm.org/D51964





More information about the llvm-commits mailing list