[PATCH] D24700: [InstCombine] optimize unsigned icmp of inc/dec like signed

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 19 12:51:58 PDT 2016


sanjoy added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2832
@@ +2831,3 @@
+
+  // icmp uge (X + -1), Y -> icmp ugt X, Y
+  if (A && NoOp0WrapProblem && Pred == CmpInst::ICMP_UGE &&
----------------
Deewiant wrote:
> sanjoy wrote:
> > This looks wrong --  if `X` and `Y` are both `0` then `uge (X +nuw -1) Y` is `uge -1 0` == `true` but `ugt X Y` is `ugt 0 0` = `false`.  Same for some of the cases below.
> D'oh, you're right. I guess this relates to what I said initially about `X +nuw -1` vs `X -nuw 1`: unless I screwed up even more, these properties do hold for subtractions, but we canonicalize them to nuwless additions so it wouldn't be very useful to match them. Would you rather have me remove these cases entirely or do the currently-useless subtraction matching in the hopes that the canonicalization is changed? I can also do that canonicalization change in `InstCombineAddSub` but I'm worried that it might have far-reaching effects.
I'd suggest removing these for now, and adding a TODO about supporting subtractions later if the canonicalization changes.



https://reviews.llvm.org/D24700





More information about the llvm-commits mailing list