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

Migrate J9 specific Linkages to appropriate J9 namespace #7768

Merged
merged 17 commits into from
Nov 21, 2019

Conversation

0xdaryl
Copy link
Contributor

@0xdaryl 0xdaryl commented Nov 14, 2019

Includes JNI, Helper, CHelper, and Private linkages specific to OpenJ9.

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Nov 14, 2019

Jenkins test sanity all jdk11

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Nov 14, 2019

@ymanton @dsouzai @fjeremic : I would appreciate a review and merge from at least one of you please (when a full set of CI builds can be run).

This is just some cleanup work to move the various J9 linkages into the appropriate namespace. The changes should be fairly straightforward.

@fjeremic fjeremic self-assigned this Nov 14, 2019
@DanHeidinga
Copy link
Member

@0xdaryl Is there a design that describes the new(?) namespace approach? It would be good to document what's changing here and why.

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Nov 15, 2019

Namespaces in the compiler were introduced when the language-agnostic compiler technology (i.e., OMR) was separated from the Java JIT (i.e., OpenJ9). They are used not only to provide project-level separation of code, but for the correct operation of the extensible class mechanism employed by the OMR compiler and Java JIT.

There isn't a lot of documentation on the use of namespaces in the compiler, but there is some provided within OMR:

https://github.com/eclipse/omr/tree/master/compiler#namespaces

Further info on extensible classes and its use of namespaces is here:

https://github.com/eclipse/omr/blob/master/doc/compiler/extensible_classes/Extensible_Classes.md#naming-and-namespaces

When OMR and OpenJ9 were open sourced, there was a lot of refactoring work still to be done that was left as an exercise to be completed over time as resources permitted. One of those pieces of work was to continue to move code into the appropriate namespaces in both the OMR and OpenJ9 projects. The work in this PR does that for one single class hierarchy (the Linkage hierarchy) in OpenJ9. It makes the class names and file names consistent across all architectures.

Removing the TR_ smurfing and making proper use of the TR:: namespace is also an ongoing goal (eclipse/omr#3519).

@fjeremic
Copy link
Contributor

Our x86 infrastructure is still offline. I'd prefer to wait until Monday until those issues are resolved so we can re-launch those builds and ensure no breakage if that is ok.

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Nov 15, 2019

Sure, that's no problem. Thanks.

@fjeremic
Copy link
Contributor

Jenkins test sanity all jdk11

Rename to `JNILinkage` as PPC prefix is redundant.

Fix up include directive paths as well.

Signed-off-by: Daryl Maier <[email protected]>
Also, rename to `JNILinkage` as the J9S390 prefix is redundant.

Signed-off-by: Daryl Maier <[email protected]>
Rename to `JNILinkage` as ARM64 prefix is redundant.

Signed-off-by: Daryl Maier <[email protected]>
Rename to `JNILinkage` as ARM prefix is redundant.

Signed-off-by: Daryl Maier <[email protected]>
* Migrate IA32JNILinkage to `J9::X86::I386::JNILinkage`
* Migrate AMD64JNILinkage to `J9::X86::AMD64::JNILinkage`

Signed-off-by: Daryl Maier <[email protected]>
* Remove unused J9S390PrivateLinkage.hpp
* Correct path to S390PrivateLinkage.cpp in CMakeLists.txt

Signed-off-by: Daryl Maier <[email protected]>
Rename J9S390CHelperLinkage.cpp -> S390CHelperLinkage.cpp
Rename J9S390CHelperLinkage.hpp -> S390CHelperLinkage.hpp

Signed-off-by: Daryl Maier <[email protected]>
* J9::X86PrivateLinkage -> J9::X86::PrivateLinkage
* J9::IA32PrivateLinkage -> J9::X86::I386::PrivateLinkage
* J9::AMD64PrivateLinkage -> J9::X86::AMD64::PrivateLinkage
* remove unused `toIA32PrivateLinkage` helper function

Signed-off-by: Daryl Maier <[email protected]>
Also migrate TR::X86HelperCallSite to J9::X86 namespace.

Signed-off-by: Daryl Maier <[email protected]>
@0xdaryl
Copy link
Contributor Author

0xdaryl commented Nov 20, 2019

Jenkins test sanity all jdk11

@fjeremic fjeremic merged commit d5c30d6 into eclipse-openj9:master Nov 21, 2019
@knn-k
Copy link
Contributor

knn-k commented Nov 22, 2019

I am seeing a build break with AArch64, which seems to be related to this PR:

/root/openj9-openjdk-jdk11/build/linux-aarch64-normal-server-release/vm/compiler/../compiler/aarch64/codegen/ARM64PrivateLinkage.cpp:47:69: error: ISO C++ forbids declaration of 'ARM64PrivateLinkage' with no type [-fpermissive]
 J9::ARM64::PrivateLinkage::ARM64PrivateLinkage(TR::CodeGenerator *cg)
                                                                     ^
/root/openj9-openjdk-jdk11/build/linux-aarch64-normal-server-release/vm/compiler/../compiler/aarch64/codegen/ARM64PrivateLinkage.cpp:47:69: error: no 'int J9::ARM64::PrivateLinkage::ARM64PrivateLinkage(TR::CodeGenerator*)' member function declared in class 'J9::ARM64::PrivateLinkage'
make[6]: *** [/root/openj9-openjdk-jdk11/build/linux-aarch64-normal-server-release/vm/compiler/../objs/compiler/aarch64/codegen/ARM64PrivateLinkage.o] Error 1

See #7837.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants