-
Notifications
You must be signed in to change notification settings - Fork 754
Update Unsafe.copyMemory transformation to support offheap #19634
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
Conversation
16622e7
to
5b24adf
Compare
5b24adf
to
dd3ccc0
Compare
Just pushed the changes to:
Will look up the review feedback next. PR now depends on eclipse-omr/omr#7391 |
10815b6
to
fb1363c
Compare
fb1363c
to
9626109
Compare
bbe37ab
to
387a56c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all of your edits. One more round of comments - hopefully the last 😅
Oh, could I also bother you to add a comment to the code explaining why all interface types in signatures allow arrays (not just the interfaces actually implemented by arrays)? I don't want somebody to come across it and change it thinking that it's being overly conservative |
387a56c
to
fbaee3f
Compare
Pushed requested changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a problem with the line ending check now, but otherwise I'm just waiting to hear about what's going on with lowerTrees()
Found issue, not sure why... the change to use loading original symRef is causing the issue.
Trees before:
After:
Its using the wrong register (r3 instead of r31/r31) for loading src/dest in the nullCheck and adjust trees |
It's made inappropriate copies of the |
a438d22
to
142b7c0
Compare
The transformation is performed in lowerTrees and VP, given that the transformation changes control flow its in VP DelayedTransformations
142b7c0
to
01f382c
Compare
Thank you @jdmpapin for all the help and suggestions. |
I'm guessing that these latest changes actually fixed the problem with |
Yes 👍 |
Running limited testing because all changes are guarded by
At the moment, the guarded changes will not be exercised or even built in a PR build anyway Jenkins test sanity xlinux jdk21 |
TODO
doDelayedTransformations
Depends on eclipse-omr/omr#7391
Example transformation trees
Before:
After: