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 support for generating SVM relocations on Power #4777

Merged
merged 4 commits into from
Feb 22, 2019

Conversation

aviansie-ben
Copy link
Contributor

This PR fixes a number of problematic sections of the Power codegen that did not properly handle being run under AOT. While these sections were previously not run under AOT due to the limitations of AOT in general, they can run when SVM AOT is being used. These sections have been fixed up to properly handle adding AOT relocations when run under SVM AOT.

@dsouzai
Copy link
Contributor

dsouzai commented Feb 19, 2019

@gita-omr @IBMJimmyk could you guys please review?

runtime/compiler/p/codegen/CallSnippet.cpp Show resolved Hide resolved
runtime/compiler/p/codegen/CallSnippet.cpp Outdated Show resolved Hide resolved
runtime/compiler/p/codegen/CallSnippet.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@gita-omr gita-omr left a comment

Choose a reason for hiding this comment

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

Could you please provide more details on the nature of the changes? This description does not really say much. I meant the changes for the object allocation.

Previously, it was generally assumed that any method calls generated
under AOT would be unresolved calls. While this was historically true,
it is no longer true when the Symbol Validation Manager is enabled. The
Power codegen will now no longer force unresolved dispatch in these
cases, instead using the new functionality to allow resolved dispatch.

Signed-off-by: Ben Thomas <[email protected]>
Inlining of instanceof and checkcast tests in the Power codegen when
running with the Symbol Validation Manager enabled has now been fixed to
work correctly. This includes adding the correct validation records for
profiled classes and single-implementer classes.

Signed-off-by: Ben Thomas <[email protected]>
When evaluating an ArrayStoreCHK node, we add a test to see if the array
being stored into is an Object[]. When storing to an Object[], there's
no need to check the element's class, as all objects can be stored in an
Object[]. However, the address for the Object class was being loaded
without adding any relocations under AOT, which could cause incorrect
behaviour. This has been fixed under SVM AOT, while the fast path is no
longer generated under old AOT.

Signed-off-by: Ben Thomas <[email protected]>
Object allocation inlining in the Power codegen now works correctly when
the Symbol Validation Manager is enabled. The previous code was making a
number of incorrect assumptions about what would happen under AOT, which
have now been corrected.

Specifically, when allocating an array, the old AOT infrastructure would
put the address of the component class in classReg and the array class
would be retrieved from that at runtime. Under SVM AOT, it is now
possible to directly load the address of the array class, so classReg
will now contain the address of the array class and the runtime
retrieval of the array class from the component class is no longer
necessary.

Signed-off-by: Ben Thomas <[email protected]>
@aviansie-ben
Copy link
Contributor Author

I've clarified a bit about the object allocation inlining changes in the description of the relevant commit. All other issues brought up have also been resolved.

@gita-omr
Copy link
Contributor

Jenkins test sanity,extended aix,plinux jdk8,jdk11

@dsouzai
Copy link
Contributor

dsouzai commented Feb 21, 2019

Jenkins may not be available until sometime tomorrow afternoon (CET) because of maintenance.

@dsouzai
Copy link
Contributor

dsouzai commented Feb 21, 2019

Jenkins test sanity,extended aix,plinux jdk8,jdk11

can't do both sanity and extended in one go

@dsouzai
Copy link
Contributor

dsouzai commented Feb 21, 2019

Jenkins test sanity aix,plinux jdk8,jdk11

@dsouzai
Copy link
Contributor

dsouzai commented Feb 21, 2019

Jenkins test extended aix,plinux jdk8,jdk11

@dsouzai
Copy link
Contributor

dsouzai commented Feb 21, 2019

extended jdk11 inux_ppc-64_cmprssptrs_le build failed with

17:08:04  + make compile
17:08:04  settings.mk:46: /home/jenkins/jenkins-agent/workspace/PullRequest-Extended-JDK11-linux_ppc-64_cmprssptrs_le-OpenJ9/openj9/test/TestConfig/../TestConfig/utils.mk: No such file or directory
17:08:04  makefile:39: count.mk: No such file or directory
17:08:04  make: *** No rule to make target 'count.mk'.  Stop.

@dsouzai
Copy link
Contributor

dsouzai commented Feb 21, 2019

Jenkins test extended plinux jdk11

@gita-omr
Copy link
Contributor

Looks like Jenkins is still not working?

@gita-omr
Copy link
Contributor

Jenkins test sanity,extended aix,plinux jdk8,jdk11

@gita-omr
Copy link
Contributor

Jenkins test sanity plinux jdk8,jdk11

@aviansie-ben
Copy link
Contributor Author

Looks like the extended test AIX failure is a known issue unrelated to these changes: #3081
As for the extended PPC64LE test failure, it's also been noted as an infra issue that should be corrected now: #4830

@gita-omr gita-omr merged commit 394debd into eclipse-openj9:master Feb 22, 2019
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.

4 participants