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

AArch64: Implement UnresolvedDataSnippet #5985

Merged
merged 1 commit into from
Jan 29, 2020
Merged

AArch64: Implement UnresolvedDataSnippet #5985

merged 1 commit into from
Jan 29, 2020

Conversation

knn-k
Copy link
Contributor

@knn-k knn-k commented Jun 3, 2019 •
edited
Loading

This commit implements UnresolvedDataSnippet and related functions for
AArch64.

Signed-off-by: knn-k konno@jp.ibm.com

@knn-k
Copy link
Contributor Author

knn-k commented Jun 3, 2019 •
edited
Loading

This depends on #5984 and eclipse-omr/omr#3938.

@knn-k
Copy link
Contributor Author

knn-k commented Jun 5, 2019 •
edited
Loading

This depends on eclipse-omr/omr#3953, too.

(Edit) This depends on #6051 instead now.

@knn-k knn-k force-pushed the aarch64runtime8 branch 3 times, most recently from 3971c79 to 93f3e90 Compare June 10, 2019 06:01
@knn-k knn-k changed the title WIP: AArch64: Implement UnresolvedDataSnippet AArch64: Implement UnresolvedDataSnippet Jun 10, 2019
@0xdaryl 0xdaryl self-assigned this Jun 12, 2019
@knn-k
Copy link
Contributor Author

knn-k commented Jul 8, 2019

Rebased to the latest code.

0xdaryl
0xdaryl reviewed Jul 22, 2019
View reviewed changes
Copy link
Contributor

@0xdaryl 0xdaryl left a comment

Choose a reason for hiding this comment

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

I'm not done reviewing, but I wanted to push these comments out.

runtime/compiler/aarch64/runtime/PicBuilder.spp Outdated
.set J9TR_UDSnippet_codeCacheReturnAddress, 0
.set J9TR_UDSnippet_CPIndex, 4
.set J9TR_UDSnippet_CP, 8
.set J9TR_UDSnippet_offset, 12
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't these offsets be: 0, 8, 12, and 20 to match the snippet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. Corrected.

runtime/compiler/aarch64/runtime/PicBuilder.spp Outdated
// movk x9, #0, LSL #16
// movk x9, #0, LSL #32
// movk x9, #0, LSL #48
// use x9 for data access
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize this code was inspired from the ARM implementation where it uses a hard-coded gr3 in the mainline code to hold the resolved address, but even in that world, how does it actually reserve gr3 for this purpose? I would think every unresolved memory reference on every instruction would need a register dependency on gr3 to prevent gr3 from being assigned to a virtual register (and hence be clobbered by the resolution code). I can't see how that is done. You'll have a similar problem with r9 on AArch64.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, the ARM implementation uses add instructions (with shifted offset) to build up the address using the base register of the memory reference (whatever that base register happens to be). If you are using x9 then my question still remains on how you plan on ensuring that register will be free.

@knn-k knn-k changed the title AArch64: Implement UnresolvedDataSnippet WIP: AArch64: Implement UnresolvedDataSnippet Aug 5, 2019
@knn-k
Copy link
Contributor Author

knn-k commented Aug 5, 2019

I am changing this from the dedicated register (x9) approach to another approach.

@knn-k knn-k mentioned this pull request Aug 22, 2019
Merged
@knn-k
Copy link
Contributor Author

knn-k commented Sep 13, 2019

Just resolved merge conflicts in J9UnresolvedDataSnippet.cpp, and fixed the check with clinit bit (Related fix: #7070).
Not ready for review yet.

@knn-k
Copy link
Contributor Author

knn-k commented Sep 18, 2019

Update the code for clinit bit checks (tst + bne to tbnz).

@knn-k knn-k mentioned this pull request Sep 26, 2019
Closed
@knn-k
Copy link
Contributor Author

knn-k commented Oct 28, 2019

Updated the files based on the another approach. Not ready for review yet.

@knn-k knn-k force-pushed the aarch64runtime8 branch 2 times, most recently from f269112 to fc7d09b Compare October 30, 2019 02:40
@knn-k
Copy link
Contributor Author

knn-k commented Oct 30, 2019

Resolved merge conflicts.

@knn-k knn-k force-pushed the aarch64runtime8 branch 3 times, most recently from 5888de2 to 088fa1c Compare November 5, 2019 06:01
@knn-k
Copy link
Contributor Author

knn-k commented Nov 5, 2019

This PR depends on eclipse-omr/omr#4225.

@knn-k
Copy link
Contributor Author

knn-k commented Nov 15, 2019

Updated the L_UDclinitCase path in PicBuilder.spp.

@knn-k knn-k force-pushed the aarch64runtime8 branch 2 times, most recently from 11bbb3e to 069c723 Compare November 20, 2019 10:30
@knn-k
Copy link
Contributor Author

knn-k commented Nov 20, 2019

Updated along with the change in eclipse-omr/omr#4225, renaming modBase to extraRegister.

@knn-k knn-k changed the title WIP: AArch64: Implement UnresolvedDataSnippet AArch64: Implement UnresolvedDataSnippet Nov 26, 2019
@knn-k
Copy link
Contributor Author

knn-k commented Nov 26, 2019

Removed "WIP:" as eclipse-omr/omr#4225 was merged.

@knn-k
Copy link
Contributor Author

knn-k commented Dec 5, 2019

Rebased and resolved the merge conflict.

@knn-k
AArch64: Implement UnresolvedDataSnippet
bdf8645 
This commit implements UnresolvedDataSnippet and related functions for
AArch64.

Signed-off-by: knn-k <konno@jp.ibm.com>
@knn-k
Copy link
Contributor Author

knn-k commented Dec 10, 2019

Added the following four entries to PicBuilder.spp and Runtime.cpp:

_interpreterUnresolvedMethodTypeGlue
_interpreterUnresolvedMethodHandleGlue
_interpreterUnresolvedCallSiteTableEntryGlue
_interpreterUnresolvedMethodTypeTableEntryGlue

@0xdaryl
Copy link
Contributor

0xdaryl commented Jan 29, 2020

Jenkins compile alinux64xl jdk11

@0xdaryl 0xdaryl merged commit c97a9a1 into eclipse-openj9:master Jan 29, 2020
@knn-k knn-k deleted the aarch64runtime8 branch January 30, 2020 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xdaryl 0xdaryl 0xdaryl approved these changes

Assignees

@0xdaryl 0xdaryl

Labels
arch:aarch64 comp:jit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@knn-k @0xdaryl