[PATCH] D24700: [InstCombine] optimize unsigned icmp of increment

[Matti Niemenmaa via llvm-commits]

Deewiant added inline comments.

================
Comment at: test/Transforms/InstCombine/icmp.ll:2799
@@ +2798,3 @@
+; CHECK-LABEL: @cmp_ult_rhs_inc(
+; CHECK-NEXT:    [[CONV:%.*]] = fptosi float %x to i32
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ule i32 [[CONV]], %i
----------------
spatel wrote:
> spatel wrote:
> > sanjoy wrote:
> > > Ok.
> > The commuting happens because of operand complexity canonicalization. It would be nice to add a comment above here to make that clear.
> > 
> > Please see:
> > https://llvm.org/bugs/show_bug.cgi?id=28296 
> > https://reviews.llvm.org/D24419
> > ...for more info. 
> Also note that fptosi is a unary operator, so if we fix PR28296, these tests will silently stop testing the cases that they were hoping to test (sigh). 
> 
> It might be better if they used a non-nuw 'add' to match the complexity of 'add nuw'.
If the canonicalization starts working differently the tests would rather start failing, since the `icmp` CHECKs would no longer match due to the reversed comparison.

Should I duplicate the comment next to every `fptosi`-using test here? There are 10 of them altogether.


https://reviews.llvm.org/D24700