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

Modify SCC frontend to take class parameter #18847

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

cjjdespres
Copy link
Contributor

@cjjdespres cjjdespres commented Jan 29, 2024

This is a resubmission of #18821. These changes eliminate the direct use of offsetInSharedCacheFromROMClass during compilation in order to support #18301, as this query is difficult to fulfill using information available at the server when compiling for the JITServer AOT cache and a local SCC is not available.

The previous PR caused test failures because the method definition in that PR corresponding to this one here:

virtual bool isClassInSharedCache(J9Class *clazz, uintptr_t *cacheOffset = NULL)
      { return isClassInSharedCache(reinterpret_cast<TR_OpaqueClassBlock *>(clazz), cacheOffset); }

was

virtual bool isClassInSharedCache(J9Class *clazz, uintptr_t *cacheOffset = NULL)
      { return isClassInSharedCache(reinterpret_cast<TR_OpaqueClassBlock *>(clazz)); }

which had the effect of not updating the value pointed to by cacheOffset in the JITServer AOT deserializer.

@cjjdespres cjjdespres requested a review from dsouzai as a code owner January 29, 2024 23:54
@cjjdespres
Copy link
Contributor Author

Attn @mpirvu. I know we are waiting for at least the nightly builds to pass in response to the reversion, but this is the updated PR.

I've also kept isROMClassInSharedCache public for now, as it can be useful to have this available at the client when manipulating ROM classes directly, particularly in #18839 and in my modifications to ROM class packing.

@mpirvu mpirvu self-assigned this Jan 30, 2024
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

LGTM

@mpirvu
Copy link
Contributor

mpirvu commented Jan 30, 2024

jenkins test sanity all jdk17

@mpirvu
Copy link
Contributor

mpirvu commented Jan 30, 2024

window failed due to infra

@mpirvu
Copy link
Contributor

mpirvu commented Jan 30, 2024

jenkins test sanity win jdk17

2 similar comments
@AdamBrousseau
Copy link
Contributor

jenkins test sanity win jdk17

@mpirvu
Copy link
Contributor

mpirvu commented Jan 31, 2024

jenkins test sanity win jdk17

@mpirvu mpirvu merged commit 43aa01d into eclipse-openj9:master Jan 31, 2024
13 checks passed
cjjdespres added a commit to cjjdespres/openj9 that referenced this pull request Feb 16, 2024
…-lookup"

This reverts commit 43aa01d, reversing
changes made to fe7f576.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants