[PATCH] D47922: unsigned foo(unsigned x, unsigned y) { return x > y && x != 0; } should fold to x > y

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D47922#1126459, @HLJ2009 wrote:

> In https://reviews.llvm.org/D47922#1126437, @spatel wrote:
>
> > Re: test location
> >  This transform is in InstSimplify, so it should have tests under test/Transforms/InstSimplify. Ie, we shouldn't need to run -instcombine to demonstrate the fold.
> >  Have a look in Transforms/InstSimplify/AndOrXor.ll for existing tests. You may want to do some preliminary cleanup if the tests are scattered between InstCombine and InstSimplify.
>
>
> hi ,
>
>   I found that there are several test cases not written. Do I need to add them now? Or do I write it alone and submit it again?


Please add missing tests now (before proposing a code change). That way, we will have baseline tests that show the current behavior. When your code change is made, the tests will show the improvement.

When I use Roman's suggestion to stub out the function in question, I see these are the existing positive tests for simplifyUnsignedRangeCheck():

  define i1 @and_icmp1(i32 %x, i32 %y) {
  ; CHECK-LABEL: @and_icmp1(
  ; CHECK-NEXT:    [[TMP1:%.*]] = icmp ult i32 [[X:%.*]], [[Y:%.*]]
  ; CHECK-NEXT:    ret i1 [[TMP1]]
  ;
    %1 = icmp ult i32 %x, %y
    %2 = icmp ne i32 %y, 0
    %3 = and i1 %1, %2
    ret i1 %3
  }
  
  define i1 @and_icmp2(i32 %x, i32 %y) {
  ; CHECK-LABEL: @and_icmp2(
  ; CHECK-NEXT:    ret i1 false
  ;
    %1 = icmp ult i32 %x, %y
    %2 = icmp eq i32 %y, 0
    %3 = and i1 %1, %2
    ret i1 %3
  }
  
  define i1 @or_icmp1(i32 %x, i32 %y) {
  ; CHECK-LABEL: @or_icmp1(
  ; CHECK-NEXT:    [[TMP1:%.*]] = icmp ne i32 [[Y:%.*]], 0
  ; CHECK-NEXT:    ret i1 [[TMP1]]
  ;
    %1 = icmp ult i32 %x, %y
    %2 = icmp ne i32 %y, 0
    %3 = or i1 %1, %2
    ret i1 %3
  }
  
  define i1 @or_icmp2(i32 %x, i32 %y) {
  ; CHECK-LABEL: @or_icmp2(
  ; CHECK-NEXT:    ret i1 true
  ;
    %1 = icmp uge i32 %x, %y
    %2 = icmp ne i32 %y, 0
    %3 = or i1 %1, %2
    ret i1 %3
  }
  
  define i1 @or_icmp3(i32 %x, i32 %y) {
  ; CHECK-LABEL: @or_icmp3(
  ; CHECK-NEXT:    [[TMP1:%.*]] = icmp uge i32 [[X:%.*]], [[Y:%.*]]
  ; CHECK-NEXT:    ret i1 [[TMP1]]
  ;
    %1 = icmp uge i32 %x, %y
    %2 = icmp eq i32 %y, 0
    %3 = or i1 %1, %2
    ret i1 %3
  }
  
  define i32 @or_of_zexted_icmps(i32 %i) {
  ; CHECK-LABEL: @or_of_zexted_icmps(
  ; CHECK-NEXT:    ret i32 1
  ;
    %cmp0 = icmp ne i32 %i, 0
    %conv0 = zext i1 %cmp0 to i32
    %cmp1 = icmp uge i32 4, %i
    %conv1 = zext i1 %cmp1 to i32
    %or = or i32 %conv0, %conv1
    ret i32 %or
  }


Repository:
  rL LLVM

https://reviews.llvm.org/D47922