[PATCH] D119690: [InstCombine] Update predicate when canonicalizing comparisons in canonicalizeClampLike.

[Ricky Zhou via Phabricator via llvm-commits]

rickyz added a comment.

In D119690#3474998 <https://reviews.llvm.org/D119690#3474998>, @spatel wrote:

> I didn't step through to see why the ult/uge variant doesn't trigger the bug, but there's a test now, so LGTM.

Thanks! If it helps, here is an attempt to explain why the bug is only triggered by UGT:

Pred0 = UGT

Pred0 is canonicalized to UGE here: https://github.com/llvm/llvm-project/blob/5805cfb90127195a9e8aa09716e989f286d0e22b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp#L1307-L1316. Before this change, Pred0 would still equal UGT.

https://github.com/llvm/llvm-project/blob/5805cfb90127195a9e8aa09716e989f286d0e22b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp#L1389-L1390 is supposed to flip the thresholds when the (post-canonicalization) Pred0 is UGE.

The bug does not occur for any other values of Pred0 because:
UGE is canonicalized to UGE, and correctly flips the thresholds
ULT is canonicalized to ULT, which does not require flipping the thresholds
ULE is canonicalized to ULT, which does not require flipping the thresholds

Thanks for the review - I am not an LLVM committer, so can you please land this and D119689 <https://reviews.llvm.org/D119689>? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119690