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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:1405
     // so we don't have any restrictions on C2, so we can just handle it.
+    Pred1 = ICmpInst::Predicate::ICMP_SLT;
     std::swap(ReplacementLow, ReplacementHigh);
----------------
Why set this if it is never used below here?


================
Comment at: llvm/test/Transforms/InstCombine/canonicalize-clamp-like-pattern-between-negative-and-positive-thresholds.ll:216
   %t1 = select i1 %t0, i32 %replacement_low, i32 %replacement_high
   %t2 = icmp uge i32 %x, 128
   %r = select i1 %t2, i32 %x, i32 %t1
----------------
I don't think this variation adds real coverage for the fix. 'uge' will be canonicalized to 'ugt' before we reach the buggy code, so it's the same as the previous test. 

Do we need to swap the select operands to exercise the predicate cases further? It's not clear to me from just looking at the code or the existing tests how to hit the bug.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119690