Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v8]
On Tue, 16 Feb 2021 06:24:05 GMT, Vladimir Kempik wrote: >> This is when passing a float, yes? In the case where we have more float >> arguments than n_float_register_parameters_c. >> I don't understand why you think it's acceptable to bail in this case. Can >> you explain, please? > > it's for everything that uses less than 8 bytes on a stack( ints ( 4), > shorts(2), bytes(1), floats(4)). > currently native wrapper generation does not support such cases at all, it > needs refactoring before this can be implemented. > So when a method has more argument than can be placed in registers, we may > have issues. > > So we just bailing out to interpreter in case when a smaller (<=4 b) type is > going to be passed thru the stack. > > There was attempt to implement handling such cases but currently it requires > some hacks (like using some vectors for non-specific task) - > https://github.com/openjdk/aarch64-port/pull/3 OK. I checked and the Panama preview doesn't support direct native calls for stack arguments, so we're good for now. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v8]
On Mon, 15 Feb 2021 19:07:40 GMT, Andrew Haley wrote: >> Hello, we have updated PR, now this bailout is used only by the code which >> can handle it (native wrapper generator), for the rest it will cause >> guarantee failed if this bailout is triggered > > This is when passing a float, yes? In the case where we have more float > arguments than n_float_register_parameters_c. > I don't understand why you think it's acceptable to bail in this case. Can > you explain, please? it's for everything that uses less than 8 bytes on a stack( ints ( 4), shorts(2), bytes(1), floats(4)). currently native wrapper generation does not support such cases at all, it needs refactoring before this can be implemented. So when a method has more argument than can be placed in registers, we may have issues. So we just bailing out to interpreter in case when a smaller (<=4 b) type is going to be passed thru the stack. There was attempt to implement handling such cases but currently it requires some hacks (like using some vectors for non-specific task) - https://github.com/openjdk/aarch64-port/pull/3 - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v8]
On Mon, 15 Feb 2021 18:00:50 GMT, Vladimir Kempik wrote: >> src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 839: >> >>> 837: // The code unable to handle this, bailout. >>> 838: return -1; >>> 839: #endif >> >> This looks like a bug to me. The caller doesn't necessarily check the return >> value. See CallRuntimeNode::calling_convention. > > Hello, we have updated PR, now this bailout is used only by the code which > can handle it (native wrapper generator), for the rest it will cause > guarantee failed if this bailout is triggered This is when passing a float, yes? In the case where we have more float arguments than n_float_register_parameters_c. I don't understand why you think it's acceptable to bail in this case. Can you explain, please? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v8]
On Mon, 1 Feb 2021 18:44:48 GMT, Andrew Haley wrote: >> Anton Kozlov has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 62 commits: >> >> - Merge branch 'master' into jdk-macos >> - Update copyright year for BsdAARCH64ThreadContext.java >> - Fix inclusing of StubRoutines header >> - Redo buildsys fix >> - Revert harfbuzz changes, disable warnings for it >> - Little adjustement of SlowSignatureHandler >> - Partially bring previous commit >> - Revert "Address feedback for signature generators" >> >>This reverts commit 50b55f6684cd21f8b532fa979b7b6fbb4613266d. >> - Refactor CDS disabling >> - Redo builsys support for aarch64-darwin >> - ... and 52 more: >> https://git.openjdk.java.net/jdk/compare/8a9004da...b421e0b4 > > src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 839: > >> 837: // The code unable to handle this, bailout. >> 838: return -1; >> 839: #endif > > This looks like a bug to me. The caller doesn't necessarily check the return > value. See CallRuntimeNode::calling_convention. Hello, we have updated PR, now this bailout is used only by the code which can handle it (native wrapper generator), for the rest it will cause guarantee failed if this bailout is triggered - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v8]
On Sun, 31 Jan 2021 20:14:01 GMT, Anton Kozlov wrote: >> 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) > > Anton Kozlov has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 62 commits: > > - Merge branch 'master' into jdk-macos > - Update copyright year for BsdAARCH64ThreadContext.java > - Fix inclusing of StubRoutines header > - Redo buildsys fix > - Revert harfbuzz changes, disable warnings for it > - Little adjustement of SlowSignatureHandler > - Partially bring previous commit > - Revert "Address feedback for signature generators" > >This reverts commit 50b55f6684cd21f8b532fa979b7b6fbb4613266d. > - Refactor CDS disabling > - Redo builsys support for aarch64-darwin > - ... and 52 more: > https://git.openjdk.java.net/jdk/compare/8a9004da...b421e0b4 src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 84: > 82: // on stack. Natural alignment for types are still in place, > 83: // for example double/long should be 8 bytes alligned > 84: This comment is a bit confusing because it's no longer #ifdef APPLE. Better move it up to Line 41. src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 352: > 350: > 351: #ifdef __APPLE__ > 352: virtual void pass_byte() Please remove ```#ifdef __APPLE__``` around this region. src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 839: > 837: // The code unable to handle this, bailout. > 838: return -1; > 839: #endif This looks like a bug to me. The caller doesn't necessarily check the return value. See CallRuntimeNode::calling_convention. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v8]
On Mon, 25 Jan 2021 09:48:46 GMT, Andrew Haley wrote: >> Would you like me to do something about it now? The problem is that the >> functions of SlowSignatureHandler are subtly different, so it will be >> multiple tables, not sure how many. Such change is another candidate for a >> separate code enhancement, which I would like not to mix into the JEP >> implementation (it's already not a small change :)). But if you think it >> would be a good thing, please let me know. In another case, I'll add this to >> a queue of follow-up enhancements. > > I'm not sure it can wait. This change turns already-messy code into something > significantly messier, to the extent that it's not really good enough to go > into mainline. Hello Does this look like something in the right direction ? https://github.com/VladimirKempik/jdk/commit/c2820734f4b10148154085a70d380b8c5775fa49 - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v8]
> 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) Anton Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 62 commits: - Merge branch 'master' into jdk-macos - Update copyright year for BsdAARCH64ThreadContext.java - Fix inclusing of StubRoutines header - Redo buildsys fix - Revert harfbuzz changes, disable warnings for it - Little adjustement of SlowSignatureHandler - Partially bring previous commit - Revert "Address feedback for signature generators" This reverts commit 50b55f6684cd21f8b532fa979b7b6fbb4613266d. - Refactor CDS disabling - Redo builsys support for aarch64-darwin - ... and 52 more: https://git.openjdk.java.net/jdk/compare/8a9004da...b421e0b4 - Changes: https://git.openjdk.java.net/jdk/pull/2200/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2200=07 Stats: 3266 lines in 114 files changed: 3164 ins; 41 del; 61 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
