[PATCH] D67849: [InstCombine] (a+b) < a && (a+b) != 0 -> (0-b) < a iff a/b != 0 (PR43259)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri planned changes to this revision.
lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1084-1085
+        IsAnd &&
+        (isKnownNonZero(A, Q.DL, /*Depth=*/0, Q.AC, Q.CxtI, Q.DT) ||
+         isKnownNonZero(B, Q.DL, /*Depth=*/0, Q.AC, Q.CxtI, Q.DT)))
+      return Builder.CreateICmpULT(Builder.CreateNeg(B), A);
----------------
spatel wrote:
> I don't think this pre-condition is correct. Both values must be non-zero, not just 1?
> https://rise4fun.com/Alive/U7I
> 
> Need a test like this?
> 
> 
> 
> ```
> define i1 @t9_nonzero_offset(i8 %base, i8 %offset) {
>    %cmp = icmp slt i8 %offset, 0
>    call void @llvm.assume(i1 %cmp)
>    %adjusted = add i8 %base, %offset
>    call void @use8(i8 %adjusted)
>    %not_null = icmp ne i8 %adjusted, 0
>    %no_underflow = icmp ult i8 %adjusted, %offset
>    %r = and i1 %not_null, %no_underflow
>    ret i1 %r
> } 
> 
> ```
Uhm, we are going in circles here. :)
Yes and no, the code as-is is incorrect, but we still can fold:
https://rise4fun.com/Alive/slDe
The replacement is semi-commutative here, we just need to carefully
pick which value we will be negating - the one we know is non-zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67849