[PATCH] D63026: [InstCombine] Fold icmp eq/ne (and %x, signbit), 0 -> %x s>=/s< 0 earlier

[Huihui Zhang via Phabricator via llvm-commits]

huihuiz added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1643-1644
 
   if (!And->hasOneUse())
     return nullptr;
 
----------------
lebedev.ri wrote:
> Move after the newly added code, this restriction does not apply for the pattern in question.
The restriction "And operation having only one use" was introduced in r134379 (resolving PR10267).

When And operation has more than one use, applying the following folds will transform an equality compare into an inequality compare, without cutting done any extra instructions. Inequalities are generally more expensive.

This restriction should apply to fold "(X & signbit) ==/!= 0) --> X s>=/s< 0"and "((X & ~C) ==/!= 0) C+1 is power of 2 --> X u</u>= C+1".




================
Comment at: test/Transforms/InstCombine/shl-and-signbit-icmpeq-zero.ll:125
 
 ; Fold happened
 define i1 @scalar_shl_and_signbit_eq_extra_use_shl(i32 %x, i32 %y, i32 %z, i32* %p) {
----------------
lebedev.ri wrote:
> It didn't though (:
Actually fold happened even without this reordering.
look for the generate ll

```
  **%shl = shl i32 %x, %y**
  %xor = xor i32 %shl, %z
  store i32 %xor, i32* %p, align 4
  **%r = icmp sgt i32 %shl, -1**

```

we are trying to schedule (X & signbit) ==/!= 0) -> X s>=/s< 0; before ((X << Y) & C) == 0 -> (X & (C >> Y)) == 0.
The second fold has restriction on "shift having only one use", which failed for this case.

So fold happened even without reorder, extra use of shift doesn't restrict on the first fold


================
Comment at: test/Transforms/InstCombine/shl-and-signbit-icmpeq-zero.ll:142
 
 ; Not fold
 define i1 @scalar_shl_and_signbit_eq_extra_use_and(i32 %x, i32 %y, i32 %z, i32* %p) {
----------------
lebedev.ri wrote:
> Similarly, should fold.
Should still comply with restriction on "And operation having only one use", as explained in r134379.


================
Comment at: test/Transforms/InstCombine/shl-and-signbit-icmpeq-zero.ll:160
 
 ; Not fold
 define i1 @scalar_shl_and_signbit_eq_extra_use_shl_and(i32 %x, i32 %y, i32 %z, i32* %p, i32* %q) {
----------------
lebedev.ri wrote:
> Here too
Similar as above. And operation has extra use, should comply with the restriction.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63026