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

8253795: Implementation of JEP 391: macOS/AArch64 Port #2200

Closed
wants to merge 117 commits into from

Conversation

@AntonKozlov
Copy link
Member

@AntonKozlov AntonKozlov commented Jan 22, 2021

Please review the implementation of JEP 391: macOS/AArch64 Port.

It's heavily based on existing ports to linux/aarch64, macos/x86_64, and windows/aarch64.

Major changes are in:

  • src/hotspot/cpu/aarch64: support of the new calling convention (subtasks JDK-8253817, JDK-8253818)
  • src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with necessary adjustments (JDK-8253819)
  • src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), required on macOS/AArch64 platform. It's implemented with pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a thread, so W^X mode change relates to the java thread state change (for java threads). In most cases, JVM executes in write-only mode, except when calling a generated stub like SafeFetch, which requires a temporary switch to execute-only mode. The same execute-only mode is enabled when a java thread executes in java or native states. This approach of managing W^X mode turned out to be simple and efficient enough.
  • src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issues

  • JDK-8253795: Implementation of JEP 391: macOS/AArch64 Port
  • JDK-8253816: Support macOS W^X
  • JDK-8253817: Support macOS Aarch64 ABI in Interpreter
  • JDK-8253818: Support macOS Aarch64 ABI for compiled wrappers
  • JDK-8253819: Implement os/cpu for macOS/AArch64
  • JDK-8253839: Update tests and JDK code for macOS/Aarch64
  • JDK-8254941: Implement Serviceability Agent for macOS/AArch64
  • JDK-8255776: Change build system for macOS/AArch64
  • JDK-8262903: [macos_aarch64] Thread::current() called on detached thread

Reviewers

Contributors

  • Vladimir Kempik <vkempik@openjdk.org>
  • Bernhard Urban-Forster <burban@openjdk.org>
  • Ludovic Henry <luhenry@openjdk.org>
  • Monica Beckwith <mbeckwit@openjdk.org>

Download

To checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200
$ git checkout pull/2200

To update a local copy of the PR:
$ git checkout pull/2200
$ git pull https://git.openjdk.java.net/jdk pull/2200/head

AntonKozlov and others added 30 commits Sep 30, 2020
ZULU-17389, ZULU-18130, ZULU-18625, ZULU-18639
ZULU-17141, ZULU-18388
Co-authored-by: Bernhard Urban-Forster <beurba@microsoft.com>
Co-authored-by: Mat Carter <macarte@microsoft.com>
* Bring r18_tls back everywhere instead of r18_reserved

* Remove dup cpu_aarch

* JDK-8253457: bsd_aarch64 part

* Revert "Bring r18_tls back everywhere instead of r18_reserved"

This reverts commit 80da32e085c802d2195d5a76b4b4a89dcf15fba7.

* Revert of revert for r18_reserve, make part
This reverts commit 861304588e44451d73730de58479bb0e64bf9a3d.
…code cache on macOS"

This reverts commit 296224992ab4e7fb435387237eed810497471cb2.
ZULU-18759, ZULU-17253
Copy link
Member

@erikj79 erikj79 left a comment

Build changes look good.

Loading

Copy link
Member

@magicus magicus left a comment

Build changes still look good. Hope you can get this done now! :)

Loading

@AntonKozlov
Copy link
Member Author

@AntonKozlov AntonKozlov commented Mar 23, 2021

No, no, no! I am not suggesting you change anything else, just that
you do not define contentless macros. You might as well define it
to be something, and true is a reasonable default, that's all. It's
not terribly important, it's just good practice.

I'm quite prepared to drop this if it's holding up the port. It's a style thing, but it's not critical.

Sorry, I missed your reply.

R18_RESERVED is also defined in https://github.com/openjdk/jdk/blob/master/make/hotspot/gensrc/GensrcAdlc.gmk#L96. I think changing the value here and there would be slightly out of the scope of this PR, so I would prefer to avoid the suggested change.

The biggest argument from my side is that the current macro value is consistent with the rest of the macros in this file. For example


and
#ifndef SUPPORTS_NATIVE_CX8

But

#define COMPRESSED_CLASS_POINTERS_DEPENDS_ON_COMPRESSED_OOPS false

and
if (COMPRESSED_CLASS_POINTERS_DEPENDS_ON_COMPRESSED_OOPS) {

Loading

@AntonKozlov
Copy link
Member Author

@AntonKozlov AntonKozlov commented Mar 23, 2021

/issue 8262903

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Mar 23, 2021

@AntonKozlov
Adding additional issue to issue list: 8262903: [macos_aarch64] Thread::current() called on detached thread.

Loading

@drej1
Copy link

@drej1 drej1 commented Mar 23, 2021

Hi @drej1, thanks for making a comment in an OpenJDK project!

All comments and discussions in the OpenJDK Community must be made available under the OpenJDK Terms of Use. If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please Use "Add GitHub user drej1 for the summary.

If you are not an OpenJDK Author, Committer or Reviewer, simply check the box below to accept the OpenJDK Terms of Use for your comments.

Your comment will be automatically restored once you have accepted the OpenJDK Terms of Use.

Loading

@VladimirKempik
Copy link

@VladimirKempik VladimirKempik commented Mar 23, 2021

Loading

@theRealAph
Copy link
Contributor

@theRealAph theRealAph commented Mar 23, 2021

[ Back-porting this patch to JDK 11] depends on the will of openjdk11 maintainers to accept this (and few other, like jep-388, as we depend on it) contribution.

To the extent that 11u has fixed policies :) we definitely have a policy of accepting patches to keep 11u working on current hardware. So yes.

Loading

@VladimirKempik
Copy link

@VladimirKempik VladimirKempik commented Mar 23, 2021

[ Back-porting this patch to JDK 11] depends on the will of openjdk11 maintainers to accept this (and few other, like jep-388, as we depend on it) contribution.

To the extent that 11u has fixed policies :) we definitely have a policy of accepting patches to keep 11u working on current hardware. So yes.

@lewurm That sounds like a green flag for you and jep-388 (with its R18_RESERVED functionality) ;)

Loading

@mo-beck
Copy link
Contributor

@mo-beck mo-beck commented Mar 23, 2021

[ Back-porting this patch to JDK 11] depends on the will of openjdk11 maintainers to accept this (and few other, like jep-388, as we depend on it) contribution.

To the extent that 11u has fixed policies :) we definitely have a policy of accepting patches to keep 11u working on current hardware. So yes.

@lewurm That sounds like a green flag for you and jep-388 (with its R18_RESERVED functionality) ;)

Thanks, @theRealAph, and @VladimirKempik . We are on it!

Loading

@theRealAph
Copy link
Contributor

@theRealAph theRealAph commented Mar 23, 2021

[ Back-porting this patch to JDK 11] depends on the will of openjdk11 maintainers to accept this (and few other, like jep-388, as we depend on it) contribution.

To the extent that 11u has fixed policies :) we definitely have a policy of accepting patches to keep 11u working on current hardware. So yes.

@lewurm That sounds like a green flag for you and jep-388 (with its R18_RESERVED functionality) ;)

Thanks, @theRealAph, and @VladimirKempik . We are on it!

It's going to be tricky to do in a really clean way, given some of the weirdnesses of the ABI. However, I think there's probably a need for it

Loading

@AntonKozlov
Copy link
Member Author

@AntonKozlov AntonKozlov commented Mar 25, 2021

The JEP was targeted to JDK17. So I propose to integrate this.

Thank you all for the reviews, suggestions, discussions, and support!

Loading

@AntonKozlov
Copy link
Member Author

@AntonKozlov AntonKozlov commented Mar 25, 2021

/integrate

Loading

@openjdk openjdk bot added the sponsor label Mar 25, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Mar 25, 2021

@AntonKozlov
Your change (at version d362996) is now ready to be sponsored by a Committer.

Loading

@VladimirKempik
Copy link

@VladimirKempik VladimirKempik commented Mar 25, 2021

/sponsor

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Mar 25, 2021

@VladimirKempik @AntonKozlov Pushed as commit dbc9e4b.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment