Page MenuHomePhabricator

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

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

Details

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

Repository
rL LLVM

Event Timeline

HLJ2009 created this revision.Jun 10 2018, 11:55 PM
HLJ2009 edited the summary of this revision. (Show Details)Jun 10 2018, 11:58 PM

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.

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.

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).

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 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?

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.

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.

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.

Do you have commit privileges?

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.

This revision was automatically updated to reflect the committed changes.