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.

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

Add static symbol for constant pool address #4027

Merged
merged 1 commit into from
Jan 2, 2019

Conversation

cathyzhyi
Copy link
Contributor

Create a static symbol to represent the constant pool address for a
resolved method so that a loadaddr node can be created when the CP
address is needed in trees and AOT can relocate the address value
correctly.

Signed-off-by: Yi Zhang yizhang@ca.ibm.com

@cathyzhyi cathyzhyi force-pushed the symbol branch 2 times, most recently from ff42e00 to 3fa6476 Compare December 13, 2018 02:40
@cathyzhyi cathyzhyi changed the title Add static symbol for constant pool address WIP: Add static symbol for constant pool address Dec 13, 2018
@cathyzhyi cathyzhyi force-pushed the symbol branch 2 times, most recently from 6a7fd04 to 4792d78 Compare December 20, 2018 15:43
@cathyzhyi cathyzhyi changed the title WIP: Add static symbol for constant pool address Add static symbol for constant pool address Dec 20, 2018
Create a static symbol to represent the constant pool address for a
resolved method so that a loadaddr node can be created when the CP
address is needed in trees and AOT can relocate the address value
correctly.

Signed-off-by: Yi Zhang <yizhang@ca.ibm.com>
@andrewcraik
Copy link
Contributor

Jenkins test sanity xlinux,win,plinux jdk8,jdk11

@andrewcraik
Copy link
Contributor

FYI any concerns a @dsouzai ?

@dsouzai
Copy link
Contributor

dsouzai commented Jan 2, 2019

AOT can relocate the address value correctly.

I can't tell from just this commit whether that's true. Is there code somewhere already where you check that this loadaddr is for a CP Address and generate the appropriate relocation for it?

@cathyzhyi
Copy link
Contributor Author

I can't tell from just this commit whether that's true. Is there code somewhere already where you check that this loadaddr is for a CP Address and generate the appropriate relocation for it?

the check for CP Address is in this PR eclipse-omr/omr#3334

Copy link
Contributor

@dsouzai dsouzai left a comment

Choose a reason for hiding this comment

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

With eclipse-omr/omr#3334 the AOT side of things make sense, so LGTM.

@cathyzhyi
Copy link
Contributor Author

the failure is due to a known issue #4135

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.

LGTM

@andrewcraik andrewcraik merged commit 5f20e8e into eclipse-openj9:master Jan 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants