[PATCH] D129650: [InstCombine] change conditions for transform of sub to xor

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D129650#3650902 <https://reviews.llvm.org/D129650#3650902>, @nikic wrote:

>> As discussed in in D128123 <https://reviews.llvm.org/D128123>, converting sub to xor can cause codegen regressions, and it may not be reversible with respect to no-wrap as shown here:
>> https://alive2.llvm.org/ce/z/CgOlZU
>
> Your example seems to show that nsw can be recovered, only nuw can't be?

Oops - I was altering the constant values to see when the transform is and isn't bidirectional, and I grabbed the wrong link. The code comments don't match the code in that link.
These are the non-bidirectional examples (the mask is negative, but the subtract constant is not):
https://alive2.llvm.org/ce/z/vebvhH

In all other cases, the transform seems to be bidirectional:
https://alive2.llvm.org/ce/z/ffGPvL

Also note that there is an existing regression test (not altered by this patch) that shows the potential harm if we do the transform and lose the no-wrap:

  define i1 @test_negative_combined_sub_signed_overflow(i8 %x) {
  ; CHECK-LABEL: @test_negative_combined_sub_signed_overflow(
  ; CHECK-NEXT:    ret i1 false
  ;
    %y = sub nsw i8 127, %x
    %z = icmp slt i8 %y, -1
    ret i1 %z
  }

If we convert sub to xor first, we can't then fold the compare to false.


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

https://reviews.llvm.org/D129650