This is an archive of the discontinued LLVM Phabricator instance.

Differential D48000

[InstSimplify][NFC] Add tests for some missed optimization opportunities in simplifyUnsignedRangeCheck()
ClosedPublic

Authored by HLJ2009 on Jun 10 2018, 11:55 PM.

Details

Reviewers
spatel
craig.topper
lebedev.ri
Commits
rGc1d2177f9dcb: [InstSimplify] Add tests for missed optimizations in simplifyUnsignedRangeCheck…
rL335128: [InstSimplify] Add tests for missed optimizations in simplifyUnsignedRangeCheck…
Summary

For both operands are unsigned, add the following optimization test case.

  1. X > Y && X != 0 --> X > Y
  2. X > Y || X != 0 --> X != 0
  3. X <= Y || X != 0 --> true
  4. X <= Y || X == 0 --> X <= Y
  5. X > Y && X == 0 --> false

Diff Detail

Event Timeline

HLJ2009 created this revision.Jun 10 2018, 11:55 PM
Herald added a subscriber: llvm-commits. · View Herald TranscriptJun 10 2018, 11:55 PM
HLJ2009 edited the summary of this revision. (Show Details)Jun 10 2018, 11:58 PM
HLJ2009 added reviewers: spatel, craig.topper, lebedev.ri.
lebedev.ri mentioned this in D47922: [InstSimplify] Fix some missed optimization opportunities in simplifyUnsignedRangeCheck().Jun 11 2018, 1:13 AM
spatel added a comment.Jun 11 2018, 9:59 AM

I think we've sorted out the problem in D47922, but this patch is not correct as shown.

It should show the results (the missed optimization) that we currently get using trunk. Please update.

HLJ2009 updated this revision to Diff 150878.Jun 11 2018, 6:54 PM

submit code based on trunk branch.

Harbormaster completed remote builds in B19207: Diff 150878.Jun 11 2018, 6:55 PM
HLJ2009 added a comment.Jun 11 2018, 6:57 PM
In D48000#1128454, @spatel wrote:

I think we've sorted out the problem in D47922, but this patch is not correct as shown.

It should show the results (the missed optimization) that we currently get using trunk. Please update.

Can you help me look at my submission? I use the trunk branch code.

spatel added a comment.Jun 12 2018, 8:12 AM
In D48000#1129208, @HLJ2009 wrote:
In D48000#1128454, @spatel wrote:

I think we've sorted out the problem in D47922, but this patch is not correct as shown.

It should show the results (the missed optimization) that we currently get using trunk. Please update.

Can you help me look at my submission? I use the trunk branch code.

In this patch, you want to add new tests to trunk (the code change from D47922 should not exist).

Run utils/update_test_checks.py with these new tests. There should be at least one missed optimization in the CHECK lines (because D47922 is not applied).

HLJ2009 added a comment.Jun 12 2018, 8:21 AM
In D48000#1129641, @spatel wrote:
In D48000#1129208, @HLJ2009 wrote:
In D48000#1128454, @spatel wrote:

I think we've sorted out the problem in D47922, but this patch is not correct as shown.

It should show the results (the missed optimization) that we currently get using trunk. Please update.

Can you help me look at my submission? I use the trunk branch code.

In this patch, you want to add new tests to trunk (the code change from D47922 should not exist).

Run utils/update_test_checks.py with these new tests. There should be at least one missed optimization in the CHECK lines (because D47922 is not applied).

Yes, I want to do this. When this test file is accepted, I update D47922 again. Can I see the corresponding improvement ?

spatel added a comment.Jun 12 2018, 8:37 AM
In D48000#1129644, @HLJ2009 wrote:
In D48000#1129641, @spatel wrote:
In D48000#1129208, @HLJ2009 wrote:
In D48000#1128454, @spatel wrote:

I think we've sorted out the problem in D47922, but this patch is not correct as shown.

It should show the results (the missed optimization) that we currently get using trunk. Please update.

Can you help me look at my submission? I use the trunk branch code.

In this patch, you want to add new tests to trunk (the code change from D47922 should not exist).

Run utils/update_test_checks.py with these new tests. There should be at least one missed optimization in the CHECK lines (because D47922 is not applied).

Yes, I want to do this. When this test file is accepted, I update D47922 again. Can I see the corresponding improvement ?

I don't understand. Are you unable to update this patch on Phabricator with the current CHECK lines?

HLJ2009 added a comment.Jun 13 2018, 4:19 AM
In D48000#1129677, @spatel wrote:
In D48000#1129644, @HLJ2009 wrote:
In D48000#1129641, @spatel wrote:
In D48000#1129208, @HLJ2009 wrote:
In D48000#1128454, @spatel wrote:

I think we've sorted out the problem in D47922, but this patch is not correct as shown.

It should show the results (the missed optimization) that we currently get using trunk. Please update.

Can you help me look at my submission? I use the trunk branch code.

In this patch, you want to add new tests to trunk (the code change from D47922 should not exist).

Run utils/update_test_checks.py with these new tests. There should be at least one missed optimization in the CHECK lines (because D47922 is not applied).

Yes, I want to do this. When this test file is accepted, I update D47922 again. Can I see the corresponding improvement ?

I don't understand. Are you unable to update this patch on Phabricator with the current CHECK lines?

sorroy, I know how to submit change to phabricator, but I don't know how to use utils/update_test_checks.py to get the difference file we want and set the test baseline. I made some attempts but it seems to be incorrect.

lebedev.ri added a comment.Jun 13 2018, 4:24 AM
In D48000#1130857, @HLJ2009 wrote:
In D48000#1129677, @spatel wrote:
In D48000#1129644, @HLJ2009 wrote:
In D48000#1129641, @spatel wrote:
In D48000#1129208, @HLJ2009 wrote:
In D48000#1128454, @spatel wrote:

I think we've sorted out the problem in D47922, but this patch is not correct as shown.

It should show the results (the missed optimization) that we currently get using trunk. Please update.

Can you help me look at my submission? I use the trunk branch code.

In this patch, you want to add new tests to trunk (the code change from D47922 should not exist).

Run utils/update_test_checks.py with these new tests. There should be at least one missed optimization in the CHECK lines (because D47922 is not applied).

Yes, I want to do this. When this test file is accepted, I update D47922 again. Can I see the corresponding improvement ?

I don't understand. Are you unable to update this patch on Phabricator with the current CHECK lines?

sorroy, I know how to submit change to phabricator, but I don't know how to use utils/update_test_checks.py to get the difference file we want and set the test baseline. I made some attempts but it seems to be incorrect.

$ cd llvm
$ git checkout master # the svn trunk
$ arc patch D48000 
$ cd ../llvm-build/ # or whereever
$ ninja
$ ../llvm/utils/utils/update_test_checks.py --opt-binary ./bin/opt ../llvm/test/test/Transforms/InstSimplify/AndOrXor.ll
$ git commit --amend
$ git diff -p -U99999 master..arcpatch-D48000 > /tmp/patch.patch # i.e. from svn trunk to this commit
$ # Update this differential with that patch.
HLJ2009 updated this revision to Diff 151135.Jun 13 2018, 5:47 AM

use utils/update_test_checks.py to get the difference file we want and set the test baseline.

lebedev.ri accepted this revision.Jun 14 2018, 11:34 PM

LG.

This revision is now accepted and ready to land.Jun 14 2018, 11:34 PM
lebedev.ri retitled this revision from [InstSimplify]update simplifyUnsignedRangeCheck function's test case. to [InstSimplify][NFC] Add tests for some missed optimization opportunities in simplifyUnsignedRangeCheck().Jun 15 2018, 3:21 AM
lebedev.ri set the repository for this revision to rL LLVM.
lebedev.ri added a child revision: D47922: [InstSimplify] Fix some missed optimization opportunities in simplifyUnsignedRangeCheck().
spatel added a comment.Jun 18 2018, 4:27 PM

Do you have commit privileges?

HLJ2009 added a comment.Jun 18 2018, 5:39 PM
In D48000#1135916, @spatel wrote:

Do you have commit privileges?

No, I don't have.After code review, do we need to submit it to git itself ? Would you help me submit it to git before I apply for permission? Thanks very much.

Closed by commit rL335128: [InstSimplify] Add tests for missed optimizations in simplifyUnsignedRangeCheck… (authored by spatel). · Explain WhyJun 20 2018, 7:07 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
test/
Transforms/
InstSimplify/
57 lines

Diff 150676

test/Transforms/InstSimplify/AndOrXor.ll

Show First 20 Lines • Show All 368 Lines • ▼ Show 20 Lines;
%1 = icmp ult i32 %x, %y %1 = icmp ult i32 %x, %y
%2 = icmp ne i32 %y, 0 %2 = icmp ne i32 %y, 0
%3 = and i1 %1, %2 %3 = and i1 %1, %2
ret i1 %3 ret i1 %3
} }
define i1 @and_icmp2(i32 %x, i32 %y) { define i1 @and_icmp2(i32 %x, i32 %y) {
; CHECK-LABEL: @and_icmp2( ; CHECK-LABEL: @and_icmp2(
; CHECK-NEXT: [[TMP1:%.*]] = icmp ugt i32 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: ret i1 [[TMP1]]
;
%1 = icmp ugt i32 %x, %y
%2 = icmp ne i32 %x, 0
%3 = and i1 %1, %2
ret i1 %3
}
define i1 @and_icmp3(i32 %x, i32 %y) {
; CHECK-LABEL: @and_icmp3(
; CHECK-NEXT: ret i1 false ; CHECK-NEXT: ret i1 false
; ;
%1 = icmp ult i32 %x, %y %1 = icmp ult i32 %x, %y
%2 = icmp eq i32 %y, 0 %2 = icmp eq i32 %y, 0
%3 = and i1 %1, %2 %3 = and i1 %1, %2
ret i1 %3 ret i1 %3
} }
define i1 @and_icmp4(i32 %x, i32 %y) {
; CHECK-LABEL: @and_icmp4(
; CHECK-NEXT: ret i1 false
;
%1 = icmp ugt i32 %x, %y
%2 = icmp eq i32 %x, 0
%3 = and i1 %1, %2
ret i1 %3
}
define i1 @or_icmp1(i32 %x, i32 %y) { define i1 @or_icmp1(i32 %x, i32 %y) {
; CHECK-LABEL: @or_icmp1( ; CHECK-LABEL: @or_icmp1(
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i32 [[Y:%.*]], 0 ; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i32 [[Y:%.*]], 0
; CHECK-NEXT: ret i1 [[TMP1]] ; CHECK-NEXT: ret i1 [[TMP1]]
; ;
%1 = icmp ult i32 %x, %y %1 = icmp ult i32 %x, %y
%2 = icmp ne i32 %y, 0 %2 = icmp ne i32 %y, 0
%3 = or i1 %1, %2 %3 = or i1 %1, %2
ret i1 %3 ret i1 %3
} }
define i1 @or_icmp2(i32 %x, i32 %y) { define i1 @or_icmp2(i32 %x, i32 %y) {
; CHECK-LABEL: @or_icmp2( ; CHECK-LABEL: @or_icmp2(
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i32 [[X:%.*]], 0
; CHECK-NEXT: ret i1 [[TMP1]]
;
%1 = icmp ugt i32 %x, %y
%2 = icmp ne i32 %x, 0
%3 = or i1 %1, %2
ret i1 %3
}
define i1 @or_icmp3(i32 %x, i32 %y) {
; CHECK-LABEL: @or_icmp3(
; CHECK-NEXT: ret i1 true ; CHECK-NEXT: ret i1 true
; ;
%1 = icmp uge i32 %x, %y %1 = icmp uge i32 %x, %y
%2 = icmp ne i32 %y, 0 %2 = icmp ne i32 %y, 0
%3 = or i1 %1, %2 %3 = or i1 %1, %2
ret i1 %3 ret i1 %3
} }
define i1 @or_icmp3(i32 %x, i32 %y) { define i1 @or_icmp4(i32 %x, i32 %y) {
; CHECK-LABEL: @or_icmp3( ; CHECK-LABEL: @or_icmp4(
; CHECK-NEXT: ret i1 true
;
%1 = icmp ule i32 %x, %y
%2 = icmp ne i32 %x, 0
%3 = or i1 %1, %2
ret i1 %3
}
define i1 @or_icmp5(i32 %x, i32 %y) {
; CHECK-LABEL: @or_icmp5(
; CHECK-NEXT: [[TMP1:%.*]] = icmp uge i32 [[X:%.*]], [[Y:%.*]] ; CHECK-NEXT: [[TMP1:%.*]] = icmp uge i32 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: ret i1 [[TMP1]] ; CHECK-NEXT: ret i1 [[TMP1]]
; ;
%1 = icmp uge i32 %x, %y %1 = icmp uge i32 %x, %y
%2 = icmp eq i32 %y, 0 %2 = icmp eq i32 %y, 0
%3 = or i1 %1, %2 %3 = or i1 %1, %2
ret i1 %3 ret i1 %3
} }
define i1 @or_icmp6(i32 %x, i32 %y) {
; CHECK-LABEL: @or_icmp6(
; CHECK-NEXT: [[TMP1:%.*]] = icmp ule i32 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: ret i1 [[TMP1]]
;
%1 = icmp ule i32 %x, %y
%2 = icmp eq i32 %x, 0
%3 = or i1 %1, %2
ret i1 %3
}
; PR27869 - Look through casts to eliminate cmps and bitwise logic. ; PR27869 - Look through casts to eliminate cmps and bitwise logic.
define i32 @and_of_zexted_icmps(i32 %i) { define i32 @and_of_zexted_icmps(i32 %i) {
; CHECK-LABEL: @and_of_zexted_icmps( ; CHECK-LABEL: @and_of_zexted_icmps(
; CHECK-NEXT: ret i32 0 ; CHECK-NEXT: ret i32 0
; ;
%cmp0 = icmp eq i32 %i, 0 %cmp0 = icmp eq i32 %i, 0
%conv0 = zext i1 %cmp0 to i32 %conv0 = zext i1 %cmp0 to i32
▲ Show 20 Lines • Show All 489 Lines • Show Last 20 Lines