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

Ensure inlined method classes are dependencies #20730

Merged
merged 1 commit into from
Dec 7, 2024

Conversation

cjjdespres
Copy link
Contributor

No description provided.

@cjjdespres cjjdespres requested a review from dsouzai as a code owner December 3, 2024 04:03
@cjjdespres
Copy link
Contributor Author

Attn @mpirvu. This does not seem to affect the failure rate much, if at all, because these classes will generally be mentioned in other relo records and so be captured as dependencies there. It's still good to make sure they're counted, I think.

@mpirvu mpirvu self-assigned this Dec 3, 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 Dec 3, 2024

jenkins test sanity all jdk21

@mpirvu
Copy link
Contributor

mpirvu commented Dec 6, 2024

AIX compilation error:

16:06:08  /home/jenkins/workspace/Build_JDK21_ppc64_aix_Personal/openj9/runtime/compiler/p/codegen/J9AheadOfTimeCompile.cpp:254:118: error: too few arguments to function call, expected 5, have 4
16:06:08           acaRecord->setClassChainForInlinedMethod(reloTarget, classChainOffsetInSharedCache, self(), classChainRecord);
16:06:08           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                    ^
16:06:08  /home/jenkins/workspace/Build_JDK21_ppc64_aix_Personal/openj9/runtime/compiler/runtime/RelocationRecord.hpp:1811:7: note: 'setClassChainForInlinedMethod' declared here
16:06:08        void setClassChainForInlinedMethod(TR_RelocationTarget *reloTarget, uintptr_t classChainOffsetInSharedCache,
16:06:08        ^

@cjjdespres
Copy link
Contributor Author

I'll fix that. I also see that I missed adding a dependency in TR_RelocationRecordPointer::setClassChainForInlinedMethod, which is distinct from TR_RelocationRecordProfiledInlinedMethod::setClassChainForInlinedMethod. That should be the last place.

I tested #20770 with this change added to make sure the failures weren't a result of the lack of dependencies. I'll double-check that PR with this last dependency added.

@cjjdespres
Copy link
Contributor Author

This should be fixed. Adding that last dependency did not seem to affect anything.

@mpirvu
Copy link
Contributor

mpirvu commented Dec 6, 2024

jenkins test sanity all jdk21

@mpirvu mpirvu merged commit 5808d05 into eclipse-openj9:master Dec 7, 2024
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants