Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Stop generating deprecated unsigned opcodes of load and store from OMR #4356

Merged
merged 2 commits into from
Feb 4, 2020
Merged

Stop generating deprecated unsigned opcodes of load and store from OMR #4356

merged 2 commits into from
Feb 4, 2020

Conversation

wbh123456
Copy link
Contributor

@wbh123456 wbh123456 commented Sep 24, 2019 •
edited

Stop generating deprecated unsigned opcodes of load and store from OMR.
Will use TR_ASSERT_FATAL to ensure no downstream projects will attempt to use aforementioned opcodes in #4363.

Issue: #2657
Signed-off-by: Bohao(Aaron) Wang aaronwang0407@gmail.com

@wbh123456 wbh123456 mentioned this pull request Sep 24, 2019
Closed
@wbh123456 wbh123456 changed the title WIP:Stop generating deprecated unsigned opcodes of load and store from OMR Stop generating deprecated unsigned opcodes of load and store from OMR Sep 25, 2019
@wbh123456 wbh123456 mentioned this pull request Sep 25, 2019
Merged
@andrewcraik
Copy link
Contributor

Why have so many lines changed in the OMRIL.cpp? is it whitespace?

@wbh123456
Copy link
Contributor Author

Why have so many lines changed in the OMRIL.cpp? is it whitespace?

It is End of Line Sequence conversion problems. Fixed that in dd6d687

andrewcraik
andrewcraik suggested changes Sep 27, 2019
View reviewed changes
Copy link
Contributor

@andrewcraik andrewcraik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cases should not be removed, but should be remapped - cases can be removed when the opcode is removed.

@wbh123456
Copy link
Contributor Author

Passed internal testings on this branch with OpenJ9 head branch. Should be good for a review. @andrewcraik

@andrewcraik
Copy link
Contributor

@genie-omr build all

@wbh123456
Copy link
Contributor Author

Note that immediately after this PR is merged, eclipse-openj9/openj9#7214 should be "unwipped" and merged to prevent performance regression.

@wbh123456
remove generation of deprecated unsigned load opcodes
946b00e 
Signed-off-by: Bohao(Aaron) Wang <aaronwang0407@gmail.com>
@wbh123456
Remove generations of deprecated unsigned store opcodes
67c1af8 
Signed-off-by: Bohao(Aaron) Wang <aaronwang0407@gmail.com>
@andrewcraik
Copy link
Contributor

freshen the build status

@andrewcraik
Copy link
Contributor

@genie-omr build all

@wbh123456
Copy link
Contributor Author

wbh123456 commented Oct 21, 2019 •
edited

Note that immediately after this PR is merged, eclipse/openj9#7214 should be "unwipped" and merged to prevent performance regression.

Passed internal testings along with the OpenJ9 PR eclipse-openj9/openj9#7214.

@wbh123456
Copy link
Contributor Author

Ping @Leonardo2718 @vijaysun-omr

@andrewcraik
Copy link
Contributor

Note the coordinated delivery

@wbh123456
Copy link
Contributor Author

@andrewcraik This PR is ready along with eclipse-openj9/openj9#7214.

@0xdaryl
Copy link
Contributor

0xdaryl commented Feb 4, 2020

I'm going to merge this PR first, and once the OpenJ9 OMR Acceptance build succeeds I will merge the corresponding OpenJ9 change. I did verify that this PR (once rebased) can build successfully on its own against OpenJ9 master.

@0xdaryl 0xdaryl self-assigned this Feb 4, 2020
@0xdaryl 0xdaryl merged commit d77a173 into eclipse:master Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewcraik andrewcraik andrewcraik approved these changes

@0xdaryl 0xdaryl 0xdaryl approved these changes

@Leonardo2718 Leonardo2718 Awaiting requested review from Leonardo2718

@vijaysun-omr vijaysun-omr Awaiting requested review from vijaysun-omr vijaysun-omr is a code owner

Assignees

@0xdaryl 0xdaryl

Labels
comp:compiler
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@wbh123456 @andrewcraik @0xdaryl @Leonardo2718