Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v6]
On Wed, 22 May 2024 14:47:43 GMT, Yudi Zheng wrote: >> Moving array construction within BigInteger.implMultiplyToLen intrinsic >> candidate to its caller simplifies the intrinsic implementation in JIT >> compiler. > > Yudi Zheng has updated the pull request incrementally with one additional > commit since the last revision: > > address comments. PPC64 part and shared code looks correct. "java/math/BigInteger" tests have passed on PPC64. I didn't review the other platforms. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18226#pullrequestreview-2072434441
Re: RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null [v3]
On Fri, 24 May 2024 07:24:13 GMT, Matthias Baesken wrote: >> When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing >> jtreg tests afterwards I run into this error : >> >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime >> error: null pointer passed as argument 2, which is declared to never be null >> #0 0x7fd95bec78d8 in spawnChild >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 >> #1 0x7fd95bec78d8 in startChild >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 >> #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 >> #3 0x7fd93797a06d () >> >> this is the memcpy call getting an unexpected null pointer : >> memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. >> Something similar was discussed and fixed here >> https://bugs.python.org/issue27570 for Python . >> >> Similar issue in OpenJDK _ >> https://bugs.openjdk.org/browse/JDK-8332473 >> 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed >> as argument 1, which is declared to never be null > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > handle special case that memcpy src is NULL but a len larger than 0 was > given I also think that the `sp.dirlen > 0` check is not necessary, but I'm ok with this version, too. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19329#pullrequestreview-2077166773
RFR: 8332228: TypePollution.java: Unrecognized VM option 'UseSecondarySuperCache'
Fix obvious typo in micro benchmark. - Commit messages: - 8332228: TypePollution.java: Unrecognized VM option 'UseSecondarySuperCache' Changes: https://git.openjdk.org/jdk/pull/19427/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19427&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8332228 Stats: 8 lines in 1 file changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/19427.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19427/head:pull/19427 PR: https://git.openjdk.org/jdk/pull/19427
Re: RFR: 8332228: TypePollution.java: Unrecognized VM option 'UseSecondarySuperCache'
On Tue, 28 May 2024 14:01:44 GMT, Martin Doerr wrote: > Fix obvious typo in micro benchmark. Thanks for the review! - PR Comment: https://git.openjdk.org/jdk/pull/19427#issuecomment-2135446758
Re: RFR: 8332228: TypePollution.java: Unrecognized VM option 'UseSecondarySuperCache'
On Tue, 28 May 2024 14:01:44 GMT, Martin Doerr wrote: > Fix obvious typo in micro benchmark. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/19427#issuecomment-2136008570
Integrated: 8332228: TypePollution.java: Unrecognized VM option 'UseSecondarySuperCache'
On Tue, 28 May 2024 14:01:44 GMT, Martin Doerr wrote: > Fix obvious typo in micro benchmark. This pull request has now been integrated. Changeset: 9ac8d05a Author: Martin Doerr URL: https://git.openjdk.org/jdk/commit/9ac8d05a2567fbf65b944660739e5f8ad1fc2020 Stats: 8 lines in 1 file changed: 0 ins; 0 del; 8 mod 8332228: TypePollution.java: Unrecognized VM option 'UseSecondarySuperCache' Reviewed-by: chagedorn, kvn - PR: https://git.openjdk.org/jdk/pull/19427
Re: [jdk23] RFR: 8222884: ConcurrentClassDescLookup.java times out intermittently
On Mon, 24 Jun 2024 09:40:14 GMT, Christoph Langer wrote: > Hi all, > > This pull request contains a backport of > [JDK-8222884](https://bugs.openjdk.org/browse/JDK-8222884), commit > [bd046d9b](https://github.com/openjdk/jdk/commit/bd046d9b9e79e4eea89c72af358961ef6e98e660) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Jaikiran Pai on 12 Jun 2024 and > was reviewed by Roger Riggs and Matthias Baesken. > > Thanks! LGTM. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19853#pullrequestreview-2135601316
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v3]
On Tue, 25 Jun 2024 13:47:39 GMT, Adam Sotona wrote: >> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code >> generation and unfortunately it causes StackOverflow on BigEndian platforms. >> >> This patch converts all lambdas in ClassSpecializer into anonymous inner >> classes. >> >> Please review and test on a BigEndian platform. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > more de-lambda work on ClassSpecializer Thanks! This looks better. `jtreg:test/jdk/java/lang/invoke/condy` have passed on linux ppc64 Big Endian. I'll put it in our nightly tests to run it on AIX, too. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2189112288
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v3]
On Tue, 25 Jun 2024 13:47:39 GMT, Adam Sotona wrote: >> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code >> generation and unfortunately it causes StackOverflow on BigEndian platforms. >> >> This patch converts all lambdas in ClassSpecializer into anonymous inner >> classes. >> >> Please review and test on a BigEndian platform. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > more de-lambda work on ClassSpecializer The tests have passed on AIX, too. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2191118572
Re: RFR: 8339166: java/lang/String/concat/HiddenClassUnloading.java fails on AIX and Linux ppc64le after JDK-8336856
On Thu, 29 Aug 2024 13:55:25 GMT, Matthias Baesken wrote: > We see HiddenClassUnloading.java failing on the ppc64 based platforms. On AIX > it seems to fail always; Linux ppc64le sometimes. > Failure output : > java.lang.RuntimeException: unloadedClassCount is zero > at HiddenClassUnloading.main(HiddenClassUnloading.java:66) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) > at java.base/java.lang.reflect.Method.invoke(Method.java:573) > at > com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138) > at java.base/java.lang.Thread.run(Thread.java:1575) > > Ergonomics sets on these platforms the heap to 32M (even with the -Xmx8M > -Xms8M settings), see the Xlog UL output > > > [0.045s][info ][gc,init ] Heap Min Capacity: 32M > [0.045s][info ][gc,init ] Heap Initial Capacity: 32M > [0.045s][info ][gc,init ] Heap Max Capacity: 32M > > > So we need more iterations in the test to trigger the class unloading. LGTM. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20771#pullrequestreview-2271807783
Re: RFR: 8339364: AIX build fails: various unused variable and function warnings [v2]
On Mon, 2 Sep 2024 13:25:51 GMT, Matthias Baesken wrote:
>> We get a couple of warnings as errors on AIX because of unused variables or
>> functions , for example :
>> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:665:10:
>> error: unused variable 'exePath' [-Werror,-Wunused-variable]
>> char exePath[PATH_MAX];
>> ^
>> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:668:9:
>> error: unused variable 'ret' [-Werror,-Wunused-variable]
>> int ret;
>> ^
>> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:664:10:
>> error: unused variable 'fn' [-Werror,-Wunused-variable]
>> char fn[32];
>> ^
>>
>> This seems to be related to the recent make changes
>> 8339156: Use more fine-granular clang unused warnings
>> https://bugs.openjdk.org/browse/JDK-8339156
>> (we use clang too on AIX)
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> adjust indentation in X11Color.c
LGTM. Only minor nits.
src/java.base/unix/native/libjava/TimeZone_md.c line 63:
> 61: static const char *ETC_ENVIRONMENT_FILE = "/etc/environment";
> 62: #else
> 63: static char *isFileIdentical(char* buf, size_t size, char *pathname);
I think it should better be moved below `#if defined(__linux__) ||
defined(MACOSX)` instead of creating an else case.
src/java.desktop/unix/native/common/awt/X11Color.c line 1234:
> 1232: awt_allocate_systemrgbcolors (jint *rgbColors, int num_colors,
> 1233: AwtGraphicsConfigDataPtr awtData) {
> 1234: for (int i = 0; i < num_colors; i++)
This requires C99. Is this ok for all platforms with all supported compilers?
-
PR Review: https://git.openjdk.org/jdk/pull/20812#pullrequestreview-2276121217
PR Review Comment: https://git.openjdk.org/jdk/pull/20812#discussion_r1741226250
PR Review Comment: https://git.openjdk.org/jdk/pull/20812#discussion_r1741228253
Re: RFR: 8339364: AIX build fails: various unused variable and function warnings [v3]
On Tue, 3 Sep 2024 07:26:53 GMT, Matthias Baesken wrote: >> We get a couple of warnings as errors on AIX because of unused variables or >> functions , for example : >> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:665:10: >> error: unused variable 'exePath' [-Werror,-Wunused-variable] >> char exePath[PATH_MAX]; >> ^ >> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:668:9: >> error: unused variable 'ret' [-Werror,-Wunused-variable] >> int ret; >> ^ >> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:664:10: >> error: unused variable 'fn' [-Werror,-Wunused-variable] >> char fn[32]; >> ^ >> >> This seems to be related to the recent make changes >> 8339156: Use more fine-granular clang unused warnings >> https://bugs.openjdk.org/browse/JDK-8339156 >> (we use clang too on AIX) > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > TimeZone_md move platform stuff Thanks for the update! I also think that adding result checks is better in a separate RFE. This build fix is good to go IMHO. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20812#pullrequestreview-2276906529
Re: RFR: 8337753: Target class of upcall stub may be unloaded
On Tue, 6 Aug 2024 17:26:55 GMT, Jorn Vernee wrote: > As discussed in the JBS issue: > > FFM upcall stubs embed a `Method*` of the target method in the stub. This > `Method*` is read from the `LambdaForm::vmentry` field associated with the > target method handle at the time when the upcall stub is generated. The MH > instance itself is stashed in a global JNI ref. So, there should be a > reachability chain to the holder class: `MH (receiver) -> LF (form) -> > MemberName (vmentry) -> ResolvedMethodName (method) -> Class (vmholder)` > > However, it appears that, due to multiple threads racing to initialize the > `vmentry` field of the `LambdaForm` of the target method handle of an upcall > stub, it is possible that the `vmentry` is updated _after_ we embed the > corresponding `Method`* into an upcall stub (or rather, the latest update is > not visible to the thread generating the upcall stub). Technically, it is > fine to keep using a 'stale' `vmentry`, but the problem is that now the > reachability chain is broken, since the upcall stub only extracts the target > `Method*`, and doesn't keep the stale `vmentry` reachable. The holder class > can then be unloaded, resulting in a crash. > > The fix I've chosen for this is to mimic what we already do in > `MethodHandles::jump_to_lambda_form`, and re-load the `vmentry` field from > the target method handle each time. Luckily, this does not really seem to > impact performance. > > > Performance numbers > x64: > > before: > > Benchmark Mode Cnt Score Error Units > Upcalls.panama_blank avgt 30 69.216 ± 1.791 ns/op > > > after: > > Benchmark Mode Cnt Score Error Units > Upcalls.panama_blank avgt 30 67.787 ± 0.684 ns/op > > > aarch64: > > before: > > Benchmark Mode Cnt Score Error Units > Upcalls.panama_blank avgt 30 61.574 ± 0.801 ns/op > > > after: > > Benchmark Mode Cnt Score Error Units > Upcalls.panama_blank avgt 30 61.218 ± 0.554 ns/op > > > > As for the added TestUpcallStress test, it takes about 800 seconds to run > this test on the dev machine I'm using, so I've set the timeout quite high. > Since it runs for so long, I've dropped it from the default `jdk_foreign` > test suite, which runs in tier2. Instead the new test will run in tier4. > > Testing: tier 1-4 Can't we do these nasty loads in C++ code and use `set_vm_result_2` in `UpcallLinker::on_entry`? The GC barriers can generate excessive amounts of code with some GCs. I guess that upcalls are less performance critical, so I'd prefer the other solution. Maybe the C++ code can get optimized better, too. Some of the `DecoratorSet` should be applicable and improve performance. If that doesn't help enough, maybe we should implement a dedicated static stub? There's no need to have the code replicated in each upcall stub. Also note that each `load_heap_oop` may save and restore registers which is actually only needed once. Regarding PPC64, I think that we could avoid PRESERVATION_FRAME_LR_GP_FP_REGS if we rearrange it such that the `load_heap_oop` is done at a place where the volatile regs are not live. But seems like this optimization is not available for other platforms. Some performance related remarks: - You could use `resolve_global_jobject` which is shorter and faster and exists on most platforms. - Using `vm_result_2` is no longer needed. The Method* can be directly passed in the method reg (or loaded from `callee_target`). - If you call the stub from assembler instead of from C++ you should be able to save some extra stuff like the frame. I'll check the PPC64 code later. @offamitkumar: Can you take a look at the s390 code, please? The cross build has failed. For the future: You may want to implement `resolve_global_jobject` which is shorter and faster and available on the other platforms. src/hotspot/cpu/ppc/stubGenerator_ppc.cpp line 4778: > 4776: StubCodeMark mark(this, "StubRoutines", "upcall_stub_load_target"); > 4777: address start = __ pc(); > 4778: __ save_LR_CR(R0); I think `save_LR_CR` and `restore_LR_CR` should get removed, too. CR is not live and LR is preserved everywhere below. But, I'll check this later. - PR Comment: https://git.openjdk.org/jdk/pull/20479#issuecomment-2275524582 PR Comment: https://git.openjdk.org/jdk/pull/20479#issuecomment-2275707529 PR Comment: https://git.openjdk.org/jdk/pull/20479#issuecomment-2278240985 PR Comment: https://git.openjdk.org/jdk/pull/20479#issuecomment-2325103457 PR Review Comment: https://git.openjdk.org/jdk/pull/20479#discussion_r1713844568
Re: RFR: 8337753: Target class of upcall stub may be unloaded
On Fri, 9 Aug 2024 12:44:45 GMT, Jorn Vernee wrote: > I think that's a good compromise. (Although I wish the C++ code was just as > fast, as it's much nicer) Agreed. > I'm not sure what other decorator would apply. I think `ON_STRONG_OOP_REF` and `IS_NOT_NULL` could also be used. But, I guess that wouldn't make it fast enough. - PR Comment: https://git.openjdk.org/jdk/pull/20479#issuecomment-2278057468
Re: RFR: 8337753: Target class of upcall stub may be unloaded
On Tue, 6 Aug 2024 17:26:55 GMT, Jorn Vernee wrote: > As discussed in the JBS issue: > > FFM upcall stubs embed a `Method*` of the target method in the stub. This > `Method*` is read from the `LambdaForm::vmentry` field associated with the > target method handle at the time when the upcall stub is generated. The MH > instance itself is stashed in a global JNI ref. So, there should be a > reachability chain to the holder class: `MH (receiver) -> LF (form) -> > MemberName (vmentry) -> ResolvedMethodName (method) -> Class (vmholder)` > > However, it appears that, due to multiple threads racing to initialize the > `vmentry` field of the `LambdaForm` of the target method handle of an upcall > stub, it is possible that the `vmentry` is updated _after_ we embed the > corresponding `Method`* into an upcall stub (or rather, the latest update is > not visible to the thread generating the upcall stub). Technically, it is > fine to keep using a 'stale' `vmentry`, but the problem is that now the > reachability chain is broken, since the upcall stub only extracts the target > `Method*`, and doesn't keep the stale `vmentry` reachable. The holder class > can then be unloaded, resulting in a crash. > > The fix I've chosen for this is to mimic what we already do in > `MethodHandles::jump_to_lambda_form`, and re-load the `vmentry` field from > the target method handle each time. Luckily, this does not really seem to > impact performance. > > > Performance numbers > x64: > > before: > > Benchmark Mode Cnt Score Error Units > Upcalls.panama_blank avgt 30 69.216 ± 1.791 ns/op > > > after: > > Benchmark Mode Cnt Score Error Units > Upcalls.panama_blank avgt 30 67.787 ± 0.684 ns/op > > > aarch64: > > before: > > Benchmark Mode Cnt Score Error Units > Upcalls.panama_blank avgt 30 61.574 ± 0.801 ns/op > > > after: > > Benchmark Mode Cnt Score Error Units > Upcalls.panama_blank avgt 30 61.218 ± 0.554 ns/op > > > > As for the added TestUpcallStress test, it takes about 800 seconds to run > this test on the dev machine I'm using, so I've set the timeout quite high. > Since it runs for so long, I've dropped it from the default `jdk_foreign` > test suite, which runs in tier2. Instead the new test will run in tier4. > > Testing: tier 1-4 src/hotspot/cpu/ppc/stubGenerator_ppc.cpp line 4598: > 4596: address start = __ pc(); > 4597: > 4598: __ resolve_global_jobject(R3_ARG1, R22_tmp2, R23_tmp3, > MacroAssembler::PRESERVATION_FRAME_LR_GP_FP_REGS); // kills R31 The comment "// kills R31" is not true. Please remove it. Can you also improve the indentation in the succeeding lines, please? Otherwise, the PPC64 code looks good and the test/jdk/java/foreign tests are passing (also with ZGC). - PR Review Comment: https://git.openjdk.org/jdk/pull/20479#discussion_r1742333288
Re: RFR: JDK-8295325: tools/jlink/plugins/SaveJlinkArgfilesPluginTest.java fails on Linux ppc64le
On Fri, 14 Oct 2022 11:16:50 GMT, Matthias Baesken wrote: > The test tools/jlink/plugins/SaveJlinkArgfilesPluginTest.java fails on Linux > ppc64le because it assumes that vm.jvmci is > available on all platforms but this is currently not the case on Linux > ppc64le. > Error is : > > Error: Module jdk.internal.vm.ci not found > java.lang.module.FindException: Module jdk.internal.vm.ci not found > at java.base/java.lang.module.Resolver.findFail(Resolver.java:892) > at java.base/java.lang.module.Resolver.resolve(Resolver.java:129) > at java.base/java.lang.module.Configuration.resolve(Configuration.java:420) > at java.base/java.lang.module.Configuration.resolve(Configuration.java:254) > at > jdk.jlink/jdk.tools.jlink.internal.Jlink$JlinkConfiguration.resolve(Jlink.java:217) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.createImageProvider(JlinkTask.java:551) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.createImage(JlinkTask.java:439) > at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.run(JlinkTask.java:291) > at jdk.jlink/jdk.tools.jlink.internal.Main.run(Main.java:56) > at jdk.jlink/jdk.tools.jlink.internal.Main$JlinkToolProvider.run(Main.java:73) > at tests.JImageGenerator$JLinkTask.call(JImageGenerator.java:715) > at tests.Helper.generateDefaultImage(Helper.java:257) > at SaveJlinkArgfilesPluginTest.main(SaveJlinkArgfilesPluginTest.java:66) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) > at java.base/java.lang.reflect.Method.invoke(Method.java:578) > at > com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:312) > at java.base/java.lang.Thread.run(Thread.java:1591) Thanks for fixing it! - Marked as reviewed by mdoerr (Reviewer). PR: https://git.openjdk.org/jdk/pull/10713
Re: RFR: JDK-8299388: java/util/regex/NegativeArraySize.java fails on Alpine and sometimes Windows
On Wed, 28 Dec 2022 12:19:16 GMT, Matthias Baesken wrote: > The test java/util/regex/NegativeArraySize.java seems to have high memory > requirements, and these requirements lead to some errors. > On Alpine Linux we run regularly into this error when executing the test: > result: Failed. Unexpected exit from test [exit code: 137] > This seems to be OOM related. > Probably we should avoid running the test on Alpine. > > On Windows the test usually works, but seems to depend as well on the memory > situation of the machine. > Once we got this error recently : > OpenJDK 64-Bit Server VM warning: INFO: os::commit_memory(0x0006c000, > 5368709120, 0) failed; error='The paging file is too small for this operation > to complete' (DOS error/errno=1455) > OpenJDK 64-Bit Server VM warning: INFO: os::commit_memory(0x0006c000, > 5368709120, 0) failed; error='The paging file is too small for this operation > to complete' (DOS error/errno=1455) > result: Failed. Unexpected exit from test [exit code: 1] > > The hs_err file generated showed : > > > # There is insufficient memory for the Java Runtime Environment to continue. > # Native memory allocation (mmap) failed to map 5368709120 bytes for G1 > virtual space > # Possible reasons: > # The system is out of physical RAM or swap space > # The process is running with CompressedOops enabled, and the Java Heap may > be blocking the growth of the native heap > > > So it looks like having 5g maxMemory as a requirement is not sufficient for > the test (the reported mmap value is already slightly above 5g). Looks reasonable. Let's hear what other people think. test/jdk/java/util/regex/NegativeArraySize.java line 28: > 26: * @bug 8223174 > 27: * @summary Pattern.compile() can throw confusing > NegativeArraySizeException > 28: * @requires os.maxMemory >= 6g & vm.bits == 64 & !vm.musl Double whitespace. - Marked as reviewed by mdoerr (Reviewer). PR: https://git.openjdk.org/jdk/pull/11796
Re: RFR: 8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false
On Tue, 14 Feb 2023 14:10:08 GMT, Richard Reingruber wrote:
> This fixes the linked issue by trimming the caller of a frame to be
> deoptimized back to its `unextended_sp` iff it is compiled. The creation of
> the section `dead after deoptimization` shown in the attachment
> [yield_after_deopt_failure.log](https://bugs.openjdk.org/secure/attachment/102602/yield_after_deopt_failure.log)
> is prevented by this.
>
> A new mode is added to the test BasicExt.java where all frames are
> deoptimized after a yield operation. The issue can be deterministically
> reproduced with the new mode. It's not worth to execute all test cases with
> the new mode though. Instead `ContinuationCompiledFramesWithStackArgs_3c4` is
> always executed a 2nd time in this mode.
>
> Before this BasicExt.java was refactored for better argument processing and
> representation of the test modes.
> Also the try-catch-clause in the main method had to be changed to rethrow the
> caught exception because without this the test would have succeeded.
>
> Testing: jtreg tests tier 1-4 on standard platforms and also on ppc64le.
LGTM. Thanks for fixing!
src/hotspot/cpu/ppc/frame_ppc.cpp line 424:
> 422:
> 423: intptr_t *frame::initial_deoptimization_info() {
> 424: // `this` is the caller of the deoptee. We want to trimm it, if
> compiled, to
I believe "to trim" should contain only one 'm' in English.
-
Marked as reviewed by mdoerr (Reviewer).
PR: https://git.openjdk.org/jdk/pull/12557
Re: RFR: 8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false [v2]
On Wed, 15 Feb 2023 20:30:00 GMT, Richard Reingruber wrote: >> This fixes the linked issue by trimming the caller of a frame to be >> deoptimized back to its `unextended_sp` iff it is compiled. The creation of >> the section `dead after deoptimization` shown in the attachment >> [yield_after_deopt_failure.log](https://bugs.openjdk.org/secure/attachment/102602/yield_after_deopt_failure.log) >> is prevented by this. >> >> A new mode is added to the test BasicExt.java where all frames are >> deoptimized after a yield operation. The issue can be deterministically >> reproduced with the new mode. It's not worth to execute all test cases with >> the new mode though. Instead `ContinuationCompiledFramesWithStackArgs_3c4` >> is always executed a 2nd time in this mode. >> >> Before this BasicExt.java was refactored for better argument processing and >> representation of the test modes. >> Also the try-catch-clause in the main method had to be changed to rethrow >> the caught exception because without this the test would have succeeded. >> >> Testing: jtreg tests tier 1-4 on standard platforms and also on ppc64le. > > Richard Reingruber has updated the pull request incrementally with three > additional commits since the last revision: > > - Improve comment > - Improve comment > - Spelling Thanks! - Marked as reviewed by mdoerr (Reviewer). PR: https://git.openjdk.org/jdk/pull/12557
RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview)
Description will get added soon. - Commit messages: - Initial Panama implementation. Changes: https://git.openjdk.org/jdk/pull/12708/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8303040 Stats: 1973 lines in 58 files changed: 1865 ins; 1 del; 107 mod Patch: https://git.openjdk.org/jdk/pull/12708.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708 PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v2]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2.
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE).
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Clean fix for NativeMemorySegmentImpl issue with byteSize 0.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/4a5debfc..7315fd20
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=00-01
Stats: 6 lines in 2 files changed: 0 ins; 4 del; 2 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v2]
On Wed, 22 Feb 2023 18:31:45 GMT, Maurizio Cimadamore
wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Clean fix for NativeMemorySegmentImpl issue with byteSize 0.
>
> src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 1359:
>
>> 1357: long size = elementCount * srcElementLayout.byteSize();
>> 1358: srcImpl.checkAccess(srcOffset, size, true);
>> 1359: if (dstImpl instanceof NativeMemorySegmentImpl &&
>> dstImpl.byteSize() == 0) {
>
> As Jorn said, this change is not acceptable, as it allows bulk copy
> disregarding the segment real size. In such cases, the issue is always a
> missing unsafe resize somewhere in the linker code.
Removed this workaround. I'm glad to get rid of it.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v2]
On Wed, 22 Feb 2023 18:23:54 GMT, Jorn Vernee wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Clean fix for NativeMemorySegmentImpl issue with byteSize 0.
>
> src/java.base/share/classes/jdk/internal/foreign/PlatformLayouts.java line
> 266:
>
>> 264: * The {@code T*} native type.
>> 265: */
>> 266: public static final ValueLayout.OfAddress C_POINTER =
>> ValueLayout.ADDRESS.withBitAlignment(64);
>
> I think this is where the issue with the check in `MemorySegment::copy` comes
> from. Note how other platforms add a call to `asUnbounded` for the created
> layout, which makes any pointer boxed using this layout writable/readable
> (such as the in memory return pointer for upcalls).
Thanks for the hint! You found it pretty quickly. I had missed that when
rebasing my early prototype. Fixed.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v2]
On Thu, 23 Feb 2023 04:37:49 GMT, Martin Doerr wrote:
>> Implementation of "Foreign Function & Memory API" for linux on Power (Little
>> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>>
>> This PR does not include code for VaList support because it's supposed to
>> get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736).
>> I've kept the related tests disabled for this platform and throw an
>> exception instead. Note that the ABI doesn't precisely specify variable
>> argument lists. Instead, it refers to `` (2.2.4 Variable Argument
>> Lists).
>>
>> Big Endian support is implemented to some extend, but not complete. E.g.
>> structs with size not divisible by 8 are not passed correctly (see
>> `useABIv2` in CallArranger.java). Big Endian is excluded by selecting
>> `ARCH.equals("ppc64le")` (CABI.java) only.
>>
>> There's another limitation: This PR only accepts structures with size
>> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
>> think arbitrary sizes are not usable on other platforms, either, because
>> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2.
>>
>> The ABI has some tricky corner cases related to HFA (Homogeneous Float
>> Aggregate). The same argument may need to get passed in both, a FP reg and a
>> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
>> by the existing tests.
>>
>> I had to make changes to shared code and code for other platforms:
>> 1. Pass type information when creating `VMStorage` objects from `VMReg`.
>> This is needed for the following reasons:
>> - PPC64 ABI requires integer types to get extended to 64 bit (also see
>> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
>> know the type or at least the bit width for that.
>> - Floating point load / store instructions need the correct width to select
>> between the correct IEEE 754 formats. The register representation in single
>> FP registers is always IEEE 754 double precision on PPC64.
>> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
>> loading 4 Bytes yields different values than on Little Endian!
>> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer
>> (with byteSize() == 0) while running TestUpcallScope. Hence, existing size
>> checks don't work (see MemorySegment.java). As a workaround, I'm just
>> skipping the check in this particular case. Please check if this makes sense
>> or if there's a better fix (possibly as separate RFE).
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clean fix for NativeMemorySegmentImpl issue with byteSize 0.
> > (I'd be happy to implement the needed changes in shared code if you want,
> > since it touches `BindingSpecializer` which is pretty dense)
>
> FYI:
> [master...JornVernee:jdk:I2L](https://github.com/openjdk/jdk/compare/master...JornVernee:jdk:I2L)
> (assuming `I2L` has the same semantics as `extsw`). Then just add a
> `.cast(int.class, long.class)` wherever currently an `int` is `vmStore`d in
> the PPC CallArranger.
Correct, `extsw` performs a `I2L` conversion. I had thought about this already,
but I think my current implementation is more efficient as it combines register
moves with the 64 bit extend. Your proposal would generate separate extend and
move instructions, right?
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview)
On Wed, 22 Feb 2023 17:03:16 GMT, Jorn Vernee wrote: > I will do a more thorough review soon. Thanks a lot! > > The ABI has some tricky corner cases related to HFA (Homogeneous Float > > Aggregate). The same argument may need to get passed in both, a FP reg and > > a GP reg or stack slot (see "no partial DW rule"). This cases are not > > covered by the existing tests. > > FWIW, we have to do this for Windows vararg floats as well > ([here](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/foreign/abi/x64/windows/CallArranger.java#L231-L239)) > > This can be done by `dup`-ing the value, and using 2 `vmStore`s. (each > `vmStore` corresponding to a single register/stack location). Doing something > similar might be simpler than the `INTEGER_AND_FLOAT` and `STACK_AND_FLOAT` > storage types you're using right now. I'm not sure if that is related to the > other limitations you mention? Might be interesting to look into. (perhaps as > a separate RFE. I don't have a big issue since the current approach stays in > PPC-only code) Maybe I need to think a bit more about it. I don't really like the extra cases for `INTEGER_AND_FLOAT` and `STACK_AND_FLOAT`. On the other side, doing it in the CallArranger would break the design of factoring out the allocation from the binding generation. In addition, it seems like PPC64 is even more tricky than the Windows case. I need to pass 2 float arguments in a GP reg (or stack slot) plus one of these 2 floats in float register F13. I think this can get implemented more easily in the backend. Do you agree? - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2.
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE).
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Remove size restriction for structs. Add TODO for Big Endian.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/7315fd20..a4d844f7
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=01-02
Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Thu, 23 Feb 2023 06:18:49 GMT, Martin Doerr wrote:
>> Implementation of "Foreign Function & Memory API" for linux on Power (Little
>> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>>
>> This PR does not include code for VaList support because it's supposed to
>> get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736).
>> I've kept the related tests disabled for this platform and throw an
>> exception instead. Note that the ABI doesn't precisely specify variable
>> argument lists. Instead, it refers to `` (2.2.4 Variable Argument
>> Lists).
>>
>> Big Endian support is implemented to some extend, but not complete. E.g.
>> structs with size not divisible by 8 are not passed correctly (see
>> `useABIv2` in CallArranger.java). Big Endian is excluded by selecting
>> `ARCH.equals("ppc64le")` (CABI.java) only.
>>
>> There's another limitation: This PR only accepts structures with size
>> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
>> think arbitrary sizes are not usable on other platforms, either, because
>> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2.
>>
>> The ABI has some tricky corner cases related to HFA (Homogeneous Float
>> Aggregate). The same argument may need to get passed in both, a FP reg and a
>> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
>> by the existing tests.
>>
>> I had to make changes to shared code and code for other platforms:
>> 1. Pass type information when creating `VMStorage` objects from `VMReg`.
>> This is needed for the following reasons:
>> - PPC64 ABI requires integer types to get extended to 64 bit (also see
>> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
>> know the type or at least the bit width for that.
>> - Floating point load / store instructions need the correct width to select
>> between the correct IEEE 754 formats. The register representation in single
>> FP registers is always IEEE 754 double precision on PPC64.
>> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
>> loading 4 Bytes yields different values than on Little Endian!
>> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer
>> (with byteSize() == 0) while running TestUpcallScope. Hence, existing size
>> checks don't work (see MemorySegment.java). As a workaround, I'm just
>> skipping the check in this particular case. Please check if this makes sense
>> or if there's a better fix (possibly as separate RFE).
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove size restriction for structs. Add TODO for Big Endian.
I have removed the size restriction for structs. Passing a struct consisting of
1 char works (on Little Endian). However, passing a struct consisting of 3
chars doesn't (getting IndexOutOfBoundsException: Out of bound access on
segment MemorySegment). Neither on PPC64, nor on x86. Is that known or should I
file a bug for that?
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Thu, 23 Feb 2023 06:18:49 GMT, Martin Doerr wrote:
>> Implementation of "Foreign Function & Memory API" for linux on Power (Little
>> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>>
>> This PR does not include code for VaList support because it's supposed to
>> get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736).
>> I've kept the related tests disabled for this platform and throw an
>> exception instead. Note that the ABI doesn't precisely specify variable
>> argument lists. Instead, it refers to `` (2.2.4 Variable Argument
>> Lists).
>>
>> Big Endian support is implemented to some extend, but not complete. E.g.
>> structs with size not divisible by 8 are not passed correctly (see
>> `useABIv2` in CallArranger.java). Big Endian is excluded by selecting
>> `ARCH.equals("ppc64le")` (CABI.java) only.
>>
>> There's another limitation: This PR only accepts structures with size
>> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
>> think arbitrary sizes are not usable on other platforms, either, because
>> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2.
>>
>> The ABI has some tricky corner cases related to HFA (Homogeneous Float
>> Aggregate). The same argument may need to get passed in both, a FP reg and a
>> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
>> by the existing tests.
>>
>> I had to make changes to shared code and code for other platforms:
>> 1. Pass type information when creating `VMStorage` objects from `VMReg`.
>> This is needed for the following reasons:
>> - PPC64 ABI requires integer types to get extended to 64 bit (also see
>> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
>> know the type or at least the bit width for that.
>> - Floating point load / store instructions need the correct width to select
>> between the correct IEEE 754 formats. The register representation in single
>> FP registers is always IEEE 754 double precision on PPC64.
>> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
>> loading 4 Bytes yields different values than on Little Endian!
>> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer
>> (with byteSize() == 0) while running TestUpcallScope. Hence, existing size
>> checks don't work (see MemorySegment.java). As a workaround, I'm just
>> skipping the check in this particular case. Please check if this makes sense
>> or if there's a better fix (possibly as separate RFE).
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove size restriction for structs. Add TODO for Big Endian.
I should add tests for the tricky corner cases like the following ones:
EXPORT struct S_FF f_S_S_FF(float p0, float p1, float p2, float p3, float p4,
float p5, float p6, float p7, float p8, float p9, float p10, float p11, struct
S_FF p12, float p13) { return p12; }
EXPORT floatf_F_S_FF(float p0, float p1, float p2, float p3, float p4,
float p5, float p6, float p7, float p8, float p9, float p10, float p11, struct
S_FF p12, float p13) { return p13; }
EXPORT struct S_FF f_S_SSS_FF(struct S_FF p0, struct S_FF p1, struct S_FF
p2, struct S_FF p3, struct S_FF p4, struct S_FF p5, struct S_FF p6, float p7) {
return p6; }
EXPORT floatf_F_SSS_FF(struct S_FF p0, struct S_FF p1, struct S_FF
p2, struct S_FF p3, struct S_FF p4, struct S_FF p5, struct S_FF p6, float p7) {
return p7; }
Can I add them to the existing libraries? If so, what is the correct naming
scheme and what is needed to get them executed (adding the EXPORT alone is not
sufficient). Or should I create a separate test for these cases?
Advice will be appreciated!
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Thu, 23 Feb 2023 06:18:49 GMT, Martin Doerr wrote:
>> Implementation of "Foreign Function & Memory API" for linux on Power (Little
>> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>>
>> This PR does not include code for VaList support because it's supposed to
>> get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736).
>> I've kept the related tests disabled for this platform and throw an
>> exception instead. Note that the ABI doesn't precisely specify variable
>> argument lists. Instead, it refers to `` (2.2.4 Variable Argument
>> Lists).
>>
>> Big Endian support is implemented to some extend, but not complete. E.g.
>> structs with size not divisible by 8 are not passed correctly (see
>> `useABIv2` in CallArranger.java). Big Endian is excluded by selecting
>> `ARCH.equals("ppc64le")` (CABI.java) only.
>>
>> There's another limitation: This PR only accepts structures with size
>> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
>> think arbitrary sizes are not usable on other platforms, either, because
>> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2.
>>
>> The ABI has some tricky corner cases related to HFA (Homogeneous Float
>> Aggregate). The same argument may need to get passed in both, a FP reg and a
>> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
>> by the existing tests.
>>
>> I had to make changes to shared code and code for other platforms:
>> 1. Pass type information when creating `VMStorage` objects from `VMReg`.
>> This is needed for the following reasons:
>> - PPC64 ABI requires integer types to get extended to 64 bit (also see
>> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
>> know the type or at least the bit width for that.
>> - Floating point load / store instructions need the correct width to select
>> between the correct IEEE 754 formats. The register representation in single
>> FP registers is always IEEE 754 double precision on PPC64.
>> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
>> loading 4 Bytes yields different values than on Little Endian!
>> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer
>> (with byteSize() == 0) while running TestUpcallScope. Hence, existing size
>> checks don't work (see MemorySegment.java). As a workaround, I'm just
>> skipping the check in this particular case. Please check if this makes sense
>> or if there's a better fix (possibly as separate RFE).
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove size restriction for structs. Add TODO for Big Endian.
Some more remarks about other issues:
- Uploaded my simple reproducer to
[JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
- Using oversized load / stores is problematic. Don't forget that OpenJDK still
supports Big Endian platforms (AIX, s390x).
- Since the membar on the return path was mentioned: I think it would be good
to enable UseSystemMemoryBarrier by default on operating systems which support
it. Maybe we should discuss this with @robehn.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Fri, 24 Feb 2023 07:39:15 GMT, Robbin Ehn wrote: > > ~I don't think we've done that much testing with UseSystemMemoryBarrier > > since it was added~. I'm a bit nervous about turning it on by default since > > it's currently also used for JNI. Let's see what Robbin thinks. > > For reliability: On every Oracle tier 5 we run these test two groups: > vmTestbase_nsk_jvmti, open/test/hotspot/jtreg/:hotspot_runtime For each of > linux_x64_debug, windows_x64, linux_aarch64_debug with sysmembar on. I picked > jvmti as this is very heavy on 'JNI'. No issues reported that I know about. > We also have this test: SystemMembarHandshakeTransitionTest.java which only > JNI transisition with this options. > > For performance, the heavy lifting is on reader of thread state and the only > case I have identified is JFR. > > I suggest changing the default now, if there is issues we have time to revert > it back before RDP1. Thanks for your prompt reply! I agree, this is a good time to enable it. Do you want to do it? We'll support it and run tests on our machines and platforms. I think JFR users who want a very high sampling rate can switch it off. Making the flag diagnostic is fine with me, but that shouldn't get discussed in this PR :-) - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Fri, 24 Feb 2023 10:42:06 GMT, Robbin Ehn wrote: > > > > ~I don't think we've done that much testing with UseSystemMemoryBarrier > > > > since it was added~. I'm a bit nervous about turning it on by default > > > > since it's currently also used for JNI. Let's see what Robbin thinks. > > > > > > > > > For reliability: On every Oracle tier 5 we run these test two groups: > > > vmTestbase_nsk_jvmti, open/test/hotspot/jtreg/:hotspot_runtime For each > > > of linux_x64_debug, windows_x64, linux_aarch64_debug with sysmembar on. I > > > picked jvmti as this is very heavy on 'JNI'. No issues reported that I > > > know about. We also have this test: > > > SystemMembarHandshakeTransitionTest.java which only JNI transisition with > > > this options. > > > For performance, the heavy lifting is on reader of thread state and the > > > only case I have identified is JFR. > > > I suggest changing the default now, if there is issues we have time to > > > revert it back before RDP1. > > > > > > Thanks for your prompt reply! I agree, this is a good time to enable it. Do > > you want to do it? We'll support it and run tests on our machines and > > platforms. I think JFR users who want a very high sampling rate can switch > > it off. Making the flag diagnostic is fine with me, but that shouldn't get > > discussed in this PR :-) > > I will be skiing next week, if you want it now, I suggest you do it, > otherwise I can when I'm back. https://github.com/openjdk/jdk/pull/12753 - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Mon, 27 Feb 2023 08:49:18 GMT, Erik Österlund wrote: > I don’t think we want this to be on by default on platforms where StoreLoad > fences don't cause substantial global overheads. The benefit on such > platforms is rather low, and needing the last couple of nanoseconds of > transition speed, seems to not be a normal use case that default settings > should optimize for. Conversely, the global synchronization can be rather > intrusive, especially when it involves handshakes with N threads, and you > need to perform global synchronization across the entire machine, for each > thread poked. I would be much more afraid of that issue out of the box, than > I would be afraid of a couple of nanoseconds slower native transitions. Hi Erik, StoreLoad fences cause substantial overhead on any multi-socket system including x86_64. The benefit may be small on single-socket systems, but can the VM distinguish? We are currently looking for benchmarks which show a negative effect of enabling it. Seems like the SPEC benchmarks don't care about it. Note that we typically use only one membarrier syscall when we handshake all threads. If you know any workload which suffers, would be great to know. David is currently also checking benchmarks. We should discuss further details in https://github.com/openjdk/jdk/pull/12753. - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v4]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2.
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE).
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
HFA: Add support for nested structures. See JDK-8300294.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/a4d844f7..b461d80c
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=03
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=02-03
Stats: 37 lines in 2 files changed: 24 ins; 0 del; 13 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v5]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2.
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE).
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Minor cleanup.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/b461d80c..75b5c78f
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=04
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=03-04
Stats: 6 lines in 3 files changed: 0 ins; 2 del; 4 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Tue, 28 Feb 2023 16:54:50 GMT, Jorn Vernee wrote: >> Martin Doerr has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove size restriction for structs. Add TODO for Big Endian. > > src/hotspot/cpu/ppc/downcallLinker_ppc.cpp line 343: > >> 341: >> 342: __ flush(); >> 343: // Disassembler::decode((u_char*)start, (u_char*)__ pc(), tty); > > Leftover commented code? > > (note that the stub can also be disassembled with > `-Xlog:foreign+downcall=trace` now) Removed. Thanks for the hint! > src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 356: > >> 354: } >> 355: #endif >> 356: //blob->print_on(tty); > > Leftover commented code? Removed. - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v5]
On Wed, 22 Feb 2023 20:25:07 GMT, Jorn Vernee wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Minor cleanup.
>
> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java
> line 81:
>
>> 79: new VMStorage[] { f1, f2, f3, f4, f5, f6, f7, f8 }, // FP output
>> 80: new VMStorage[] { r0, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11,
>> r12 }, // volatile GP
>> 81: new VMStorage[] { f0, f1, f2, f3, f4, f5, f6, f7, f8, f9, f10,
>> f11, f12, f13 }, // volatile FP
>
> Note that argument registers are assumed volatile, so they don't have to be
> duplicated here.
Removed.
> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/linux/LinuxPPC64CallArranger.java
> line 33:
>
>> 31: * PPC64 CallArranger specialized for Linux ABI.
>> 32: */
>> 33: public class LinuxPPC64CallArranger extends CallArranger {
>
> I don't really see the point in having a separate subclass with
> `CallArranger` being abstract, unless you are planning to add other
> implementations later?
>
> (edit: see also later comment in CallArranger
> https://github.com/openjdk/jdk/pull/12708#discussion_r1120753657)
AIX support will need to get implemented, yet. I guess @backwaterred will work
on it.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Tue, 28 Feb 2023 19:45:28 GMT, Jorn Vernee wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove size restriction for structs. Add TODO for Big Endian.
>
> src/hotspot/cpu/ppc/downcallLinker_ppc.cpp line 133:
>
>> 131: Register callerSP= R2, // C/C++ uses R2 as TOC, but we
>> can reuse it here
>> 132:tmp = R11_scratch1, // same as shuffle_reg
>> 133:call_target_address = R12_scratch2; // same as
>> _abi._scratch2 (ABIv2 requires this reg!)
>
> Do I understand correctly that the ABI requires the register to be used for
> the call to be `R12`? How does that make a difference? I guess in some cases
> the callee might want to know the address through which it is called? (so it
> looks at `R12`)
ABI v2 requires R12 to point to the function entry point. It is used to access
constants relative to it.
> src/hotspot/cpu/ppc/downcallLinker_ppc.cpp line 154:
>
>> 152: // (abi_reg_args is abi_minframe plus space for 8 argument register
>> spill slots)
>> 153: assert(_abi._shadow_space_bytes == frame::abi_minframe_size,
>> "expected space according to ABI");
>> 154: int allocated_frame_size = frame::abi_minframe_size +
>> MAX2(_input_registers.length(), 8) * BytesPerWord;
>
> This is hard-coding an assumption about the ABI that's being called. Ok for
> now.
>
> If it needs to be addressed in the future, it could be done by adding another
> field to `ABIDescriptor` like `min_stack_arg_bytes`, or something like that
> (which is set to zero for other ABIs). It seems to be different from
> `shadow_space` since it's also used by the caller to put stack arguments.
Yeah, I think this should be done on demand.
> src/hotspot/cpu/ppc/foreignGlobals_ppc.cpp line 229:
>
>> 227:
>> 228: void ArgumentShuffle::pd_generate(MacroAssembler* masm, VMStorage tmp,
>> int in_stk_bias, int out_stk_bias, const StubLocations& locs) const {
>> 229: Register callerSP = as_Register(tmp); // preset
>
> It looks like `tmp` is being used to hold the caller's SP. I'm guessing this
> can not be computed the same way as we do on x86 and aarch64? (based on
> `RBP`, `RFP_BIAS`)
>
> If you want, you could add another register argument to `pd_generate` that is
> just invalid/unused on other platforms. That way you could use `tmp` for the
> shuffling instead of having to go through the stack. (looks like `R0` is
> already used in some cases as a temp register)
There's no BP register. It could get loaded from the back chain, but would
still need a register. R0 is always available as scratch reg, so there's no
need to pass it. (Note that I'm not going through stack because of the lack of
registers.)
Yeah, maybe passing callerSP separately would make it better readable on PPC64.
Not sure if it's worth changing.
> src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 137:
>
>> 135: ArgumentShuffle arg_shuffle(in_sig_bt, total_in_args, out_sig_bt,
>> total_out_args, &in_conv, &out_conv, shuffle_reg);
>> 136: // The Java call uses the JIT ABI, but we also call C.
>> 137: int out_arg_area = MAX2(frame::jit_out_preserve_size +
>> arg_shuffle.out_arg_bytes(), (int)frame::abi_reg_args_size);
>
> We need `frame::abi_reg_args_size` since we call `on_entry`/`on_exit` which
> require the stack space I guess?
Correct. C functions are allowed to write into the space which is part of the
caller's frame.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Tue, 28 Feb 2023 20:00:15 GMT, Jorn Vernee wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove size restriction for structs. Add TODO for Big Endian.
>
> src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 240:
>
>> 238: __ ld(call_target_address, in_bytes(Method::from_compiled_offset()),
>> R19_method);
>> 239: __ mtctr(call_target_address);
>> 240: __ bctrl();
>
> Ok, I see. I guess there is some special purpose register called `CTR` which
> we are moving to for `bctrl` here. Does ABIv2 require that move to always
> come from `R12`? (from the comment in downcallLinker).
>
> (I'm trying to understand the requirements for possibly tweaking shared code).
There's no instruction which can use a GP reg as branch target directly. That's
why we use CTR.
In this case, using R12 is not required since we call Java and our PPC64 VM
code does not rely on it.
If we were calling C, using R12 would be required by ABI v2.
> src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 347:
>
>> 345: FunctionDescriptor* fd = (FunctionDescriptor*)fd_addr;
>> 346: fd->set_entry(fd_addr + sizeof(FunctionDescriptor));
>> 347: #endif
>
> Had to do a double take. Looks like we're not the only one who are using the
> name `FunctionDescriptor` :)
Yeah, ABI v1 (Big Endian) treats function pointers as pointers to a structure
called `FunctionDescriptor` which contains the entry point plus additional
information like a pointer to a constant table.
> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java
> line 286:
>
>> 284: // "no partial DW rule": Mark first stack slot to get
>> filled.
>> 285: // Note: Can only happen with forArguments = true.
>> 286: VMStorage overlappingReg = null;
>
> `overlappingReg` is initialized along all branches, so it's not needed to
> assign `null` here (and then javac will check it is actually assigned before
> use)
Removed.
> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/TypeClass.java
> line 66:
>
>> 64: }
>> 65:
>> 66: static boolean isHomogeneousFloatAggregate(MemoryLayout type,
>> boolean useABIv2) {
>
> Note that we had to make some changes to this routine on AArch64, since it
> didn't properly account for nested structs/unions and arrays. See:
> https://github.com/openjdk/panama-foreign/pull/780
>
> Just as a heads up, in case PPC needs changes too.
Thanks for the hint! I just added the code since it is known to be needed,
small and doesn't break any of the existing tests. I'll need to test and
possibly debug the various cases, yet.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Fri, 24 Feb 2023 07:17:30 GMT, Jorn Vernee wrote: >> Some more remarks about other issues: >> - Uploaded my simple reproducer to >> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017) >> - Using oversized load / stores is problematic. Don't forget that OpenJDK >> still supports Big Endian platforms (AIX, s390x). >> - The result of `NativeCallingConvention::calling_convention` is interpreted >> as size, but it returns the max offset. That's off by one slot. Should I >> file a bug for that? (PPC64 is not affected because it doesn't use the >> result.) >> - Since the membar on the return path was mentioned: I think it would be >> good to enable UseSystemMemoryBarrier by default on operating systems which >> support it. Maybe we should discuss this with @robehn. > >> * Uploaded my simple reproducer to >> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017) > > Thanks! > >> * Using oversized load / stores is problematic. Don't forget that >> OpenJDK still supports Big Endian platforms (AIX, s390x). > > You're right. I realized that it's also problematic for heap segments, for > which we can't do oversized accesses. I am working on another solution that > splits up the loads/stores into power-of-two sized chunks: > https://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:OOB > That patch is just a POC at this point though. Also, I don't think it works > for BE at the moment (need to flip the offset for BE, I think. Just like we > do in Unsafe). > >> * The result of `NativeCallingConvention::calling_convention` is >> interpreted as size, but it returns the max offset. That's off by one slot. >> Should I file a bug for that? (PPC64 is not affected because it doesn't use >> the result.) > > I'm not sure there's an issue there. Note that the 'max offset' is computed > as `reg.offset() + reg.stack_size()`, so that should get us the size we need > to allocate for the stack arguments. (e.g. 2 ints being passed at offset 0 > and 4, would make max offset 4 + 4 = 8, which gives the size needed for the 2 > ints). Computing the max offset instead of just summing the sizes of the > stack arguments is needed since stack arguments can be sparsely placed in > some cases on Mac/AArch64. > >> * Since the membar on the return path was mentioned: I think it would be >> good to enable UseSystemMemoryBarrier by default on operating systems which >> support it. Maybe we should discuss this with @robehn. > > ~I don't think we've done that much testing with UseSystemMemoryBarrier since > it was added~. I'm a bit nervous about turning it on by default since it's > currently also used for JNI. Let's see what Robbin thinks. @JornVernee: Thanks a lot for your detailed review! I have quite a few TODOs which include: - Include my tests for the HFA corner cases. - Try to improve handling of the overlapping registers as you suggested. - Check nesting of HFA. There will surely be more when looking into Big Endian support after merging with your recent work on https://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:OOB We should get rid of oversized accesses on PPC64, too. Thanks for sharing your plans to intrisify `linkToNative` in C2 later. I guess we should do more preparation work on all platforms when that gets addressed. - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Tue, 28 Feb 2023 20:51:50 GMT, Jorn Vernee wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove size restriction for structs. Add TODO for Big Endian.
>
> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java
> line 68:
>
>> 66: public abstract class CallArranger {
>> 67: // Linux PPC64 Little Endian uses ABI v2.
>> 68: private static final boolean useABIv2 = ByteOrder.nativeOrder() ==
>> ByteOrder.LITTLE_ENDIAN;
>
> Now that I'm here. This could be a potentially interesting case for having 2
> subclasses of CallArranger: one for `useABIv2 == true` and one for `false`.
Yeah, let's wait until we know what changes we need for AIX (and Big Endian
linux).
> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java
> line 293:
>
>> 291: } else {
>> 292: overlappingReg = new
>> VMStorage(StorageType.STACK_AND_FLOAT,
>> 293:(short)
>> STACK_SLOT_SIZE, (int) stackOffset - 4);
>
> I think you could remove the mixed VMStorage types here relatively easily by
> returning a `VMStorage[][]`, where each element is a single element array,
> but then for the `needOverlapping` case add another element to the array for
> the extra store (instead of replacing the existing one).
>
> Then when unboxing a `STRUCT_HFA`, `dup` the result of the `bufferLoad` and
> then do 2 `vmStore`s (one for each element).
>
> For boxing, you could just ignore the extra storage, and just `vmLoad` the
> first one (or, whichever one you like :))
Thanks! I need to find extra time for this. Sounds like a good idea and I may
be able to get rid of some nasty code.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v6]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Add test for HFA corner cases.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/75b5c78f..c96e1120
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=05
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=04-05
Stats: 256 lines in 2 files changed: 256 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v7]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains seven commits:
- Merge branch 'master' into PPC64_Panama
- Add test for HFA corner cases.
- Minor cleanup.
- HFA: Add support for nested structures. See JDK-8300294.
- Remove size restriction for structs. Add TODO for Big Endian.
- Clean fix for NativeMemorySegmentImpl issue with byteSize 0.
- Initial Panama implementation.
-
Changes: https://git.openjdk.org/jdk/pull/12708/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=06
Stats: 2247 lines in 59 files changed: 2139 ins; 1 del; 107 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v8]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Fix merge bug.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/d71f0f0a..f23edd8e
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=07
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=06-07
Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v9]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Handle HFA corner cases with overlapping registers in Java.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/f23edd8e..98e242c2
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=08
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=07-08
Stats: 53 lines in 4 files changed: 16 ins; 20 del; 17 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Wed, 1 Mar 2023 06:26:20 GMT, Martin Doerr wrote:
>> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java
>> line 293:
>>
>>> 291: } else {
>>> 292: overlappingReg = new
>>> VMStorage(StorageType.STACK_AND_FLOAT,
>>> 293:(short)
>>> STACK_SLOT_SIZE, (int) stackOffset - 4);
>>
>> I think you could remove the mixed VMStorage types here relatively easily by
>> returning a `VMStorage[][]`, where each element is a single element array,
>> but then for the `needOverlapping` case add another element to the array for
>> the extra store (instead of replacing the existing one).
>>
>> Then when unboxing a `STRUCT_HFA`, `dup` the result of the `bufferLoad` and
>> then do 2 `vmStore`s (one for each element).
>>
>> For boxing, you could just ignore the extra storage, and just `vmLoad` the
>> first one (or, whichever one you like :))
>
> Thanks! I need to find extra time for this. Sounds like a good idea and I may
> be able to get rid of some nasty code.
Done by
https://github.com/openjdk/jdk/pull/12708/commits/98e242c24c07ea977b7709b9f8d0c10ce87e84c0
(using a record instead of a `VMStorage[][]` because I think this is better
readable). Note that it's a bit more complicated. I couldn't use your `dup`
trick, because I need to put the value into a GP reg and one half of it to a FP
reg. The Panama code doesn't support that (IllegalArgumentException: Invalid
operand type: interface java.lang.foreign.MemorySegment. float expected).
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v10]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains 10 commits:
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Handle HFA corner cases with overlapping registers in Java.
- Fix merge bug.
- Merge branch 'master' into PPC64_Panama
- Add test for HFA corner cases.
- Minor cleanup.
- HFA: Add support for nested structures. See JDK-8300294.
- Remove size restriction for structs. Add TODO for Big Endian.
- Clean fix for NativeMemorySegmentImpl issue with byteSize 0.
- Initial Panama implementation.
-
Changes: https://git.openjdk.org/jdk/pull/12708/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=09
Stats: 2287 lines in 59 files changed: 2157 ins; 16 del; 114 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v11]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Remove STRUCT_REFERENCE which was incorrectly taken from aarch64. Pass size
to bufferLoad/Store. Enable TestNested.java.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/3a541d00..f75a240d
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=10
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=09-10
Stats: 25 lines in 3 files changed: 0 ins; 21 del; 4 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v12]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
The merge change has messed up some includes in the tests. Revert.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/f75a240d..a2eed058
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=11
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=10-11
Stats: 44 lines in 6 files changed: 15 ins; 21 del; 8 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v13]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Remove LinuxPPC64CallArranger.java because it doesn't contain anything.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/a2eed058..fb87284c
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=12
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=11-12
Stats: 44 lines in 3 files changed: 0 ins; 38 del; 6 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Wed, 1 Mar 2023 06:27:19 GMT, Martin Doerr wrote:
>> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java
>> line 68:
>>
>>> 66: public abstract class CallArranger {
>>> 67: // Linux PPC64 Little Endian uses ABI v2.
>>> 68: private static final boolean useABIv2 = ByteOrder.nativeOrder() ==
>>> ByteOrder.LITTLE_ENDIAN;
>>
>> Now that I'm here. This could be a potentially interesting case for having 2
>> subclasses of CallArranger: one for `useABIv2 == true` and one for `false`.
>
> Yeah, let's wait until we know what changes we need for AIX (and Big Endian
> linux).
I think having a ABIV1CallArranger (for Big Endian linux and AIX) and a
ABIV2CallArranger (linux ppc64le) will make sense. That's why I have removed
the LinuxPPC64CallArranger.java with
https://github.com/openjdk/jdk/pull/12708/commits/fb87284c1d3df946db378d196d7f48cd3acbab01.
I guess a good time for such cleanup will be after the code for all variants
is available (not part of this PR).
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v14]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Introduce ABIv2CallArranger for linux ppc64le.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/fb87284c..2e4e269e
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=13
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=12-13
Stats: 178 lines in 8 files changed: 99 ins; 58 del; 21 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Mon, 6 Mar 2023 16:38:37 GMT, Jorn Vernee wrote: >> @JornVernee: Thanks a lot for your detailed review! I have quite a few TODOs >> which include: >> - Include my tests for the HFA corner cases. >> - Try to improve handling of the overlapping registers as you suggested. >> - Check nesting of HFA. >> >> There will surely be more when looking into Big Endian support after merging >> with your recent work on >> https://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:OOB >> We should get rid of oversized accesses on PPC64, too. >> Thanks for sharing your plans to intrisify `linkToNative` in C2 later. I >> guess we should do more preparation work on all platforms when that gets >> addressed. > > @TheRealMDoerr I've moved the support for structs/unions that are not a power > of 2 in size to this repo, so you should be able to merge the master branch > to get it now. @JornVernee: Thanks! I've merged in your changes. TestArrayStructs is not yet completely working. I will need to investigate. I think I've done most other things you had requested. You may want to take a look at my recent commits. - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Sat, 11 Mar 2023 02:20:31 GMT, Jorn Vernee wrote: >> @JornVernee: Thanks! I've merged in your changes. TestArrayStructs is not >> yet completely working. I will need to investigate. I think I've done most >> other things you had requested. You may want to take a look at my recent >> commits. > > @TheRealMDoerr I've been keeping up with the changes you're making. I just > have to look at the new test for HFA's you've added you added (next week). > > Besides fixing the TestArrayStructs test, do you have anything else you still > want to add to this PR? @JornVernee: I guess I should add a couple of Upcalls to my new test. Otherwise, I have only planned to fix `TestArrayStructs`. Further changes (and maybe new tests) can still get done when working on Big Endian / AIX or when there is a demand. I'm currently wondering about the `TestArrayStructs` failures. Passing arrays with up to 7 elements seems to work fine. When I pass 8 elements, the last element of `capturedArgs` gets observed as 0. When I pass more than 8 elements, element 5 and 6 of `capturedArgs` get observed as 0. `DowncallLinker.invokeInterpBindings` has the correct `args`, but `UpcallLinker.invokeInterpBindings` doesn't receive the correct values as `lowLevelArgs`. They contain the wrong zeros. The remaining elements look correct. Do you have an idea what could be going wrong? Otherwise, I'll have to continue debugging. - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
On Mon, 13 Mar 2023 16:37:18 GMT, Jorn Vernee wrote: > > I'm currently wondering about the TestArrayStructs failures. Passing arrays > > with up to 7 elements seems to work fine. When I pass 8 elements, the last > > element of capturedArgs gets observed as 0. When I pass more than 8 > > elements, element 5 and 6 of capturedArgs get observed as 0. > > DowncallLinker.invokeInterpBindings has the correct args, but > > UpcallLinker.invokeInterpBindings doesn't receive the correct values as > > lowLevelArgs. They contain the wrong zeros. The remaining elements look > > correct. > > Do you have an idea what could be going wrong? Otherwise, I'll have to > > continue debugging. > > This sounds like there might be a mismatch between the Java and native side. > I suggest looking at the assembly generated for the native function for the > failing case, and seeing if it matches what is generated by CallArranger. > Here is also where adding a CallArranger test can be useful (in > test/jdk/java/foreign/callarranger), to test whether the resulting bindings > match your expectation for that function descriptor. > > Also, you might want to check the layout the native compiler uses for the > particular struct, and verify that it matches the Java side. (i.e. there's no > weird padding or something, it's just a struct of 8 bytes). Note that argument and return value passing works. I'm getting all values back. So, the native side seems to be ok. Only (one or two) values in `returnBox` are broken. - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v15]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Allow TestHFA to run on musl. Add Upcalls.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/2e4e269e..364a84ed
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=14
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=13-14
Stats: 208 lines in 2 files changed: 204 ins; 1 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v14]
On Mon, 13 Mar 2023 14:31:19 GMT, Jorn Vernee wrote: >> Martin Doerr has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Introduce ABIv2CallArranger for linux ppc64le. > > test/jdk/java/foreign/TestHFA.java line 32: > >> 30: * @enablePreview >> 31: * @requires ((os.arch == "amd64" | os.arch == "x86_64") & >> sun.arch.data.model == "64") | os.arch == "aarch64" | os.arch == "ppc64le" | >> os.arch == "riscv64" >> 32: * @requires !vm.musl > > Not sure if this test should be disabled on musl? Changed with https://github.com/openjdk/jdk/pull/12708/commits/364a84edc416abd5f1318f78057c92720fe96990. Plus added Upcalls. - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v16]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Fix storing 32 bit integers into Java frames. Enable TestArrayStructs.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/364a84ed..9173af20
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=15
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=14-15
Stats: 14 lines in 2 files changed: 5 ins; 3 del; 6 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v16]
On Tue, 14 Mar 2023 22:30:22 GMT, Martin Doerr wrote:
>> Implementation of "Foreign Function & Memory API" for linux on Power (Little
>> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>>
>> This PR does not include code for VaList support because it's supposed to
>> get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736).
>> I've kept the related tests disabled for this platform and throw an
>> exception instead. Note that the ABI doesn't precisely specify variable
>> argument lists. Instead, it refers to `` (2.2.4 Variable Argument
>> Lists).
>>
>> Big Endian support is implemented to some extend, but not complete. E.g.
>> structs with size not divisible by 8 are not passed correctly (see
>> `useABIv2` in CallArranger.java). Big Endian is excluded by selecting
>> `ARCH.equals("ppc64le")` (CABI.java) only.
>>
>> There's another limitation: This PR only accepts structures with size
>> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
>> think arbitrary sizes are not usable on other platforms, either, because
>> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
>> get addressed separately:
>> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>>
>> The ABI has some tricky corner cases related to HFA (Homogeneous Float
>> Aggregate). The same argument may need to get passed in both, a FP reg and a
>> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
>> by the existing tests.
>>
>> I had to make changes to shared code and code for other platforms:
>> 1. Pass type information when creating `VMStorage` objects from `VMReg`.
>> This is needed for the following reasons:
>> - PPC64 ABI requires integer types to get extended to 64 bit (also see
>> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
>> know the type or at least the bit width for that.
>> - Floating point load / store instructions need the correct width to select
>> between the correct IEEE 754 formats. The register representation in single
>> FP registers is always IEEE 754 double precision on PPC64.
>> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
>> loading 4 Bytes yields different values than on Little Endian!
>> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer
>> (with byteSize() == 0) while running TestUpcallScope. Hence, existing size
>> checks don't work (see MemorySegment.java). As a workaround, I'm just
>> skipping the check in this particular case. Please check if this makes sense
>> or if there's a better fix (possibly as separate RFE). Update: This issue is
>> resolved by 2nd commit.
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix storing 32 bit integers into Java frames. Enable TestArrayStructs.
Thanks for the hint. Using `jshell` is a good idea.
{
callerMethodType: (long,int,int,int,int,int,int,int,int)long
calleeMethodType:
(MemorySegment,byte,byte,byte,byte,byte,byte,byte,byte)MemorySegment
FunctionDescriptor: ([[8:b8]]b8b8b8b8b8b8b8b8)[[8:b8]] Argument Bindings:
0: [Allocate[size=8, alignment=1], Dup[], VMLoad[storage=VMStorage[type=0,
segmentMaskOrSize=3, indexOrOffset=3, debugName=r3], type=long],
BufferStore[offset=0, type=long, byteWidth=8]]
1: [VMLoad[storage=VMStorage[type=0, segmentMaskOrSize=3, indexOrOffset=4,
debugName=r4], type=int], INT_TO_BYTE]
2: [VMLoad[storage=VMStorage[type=0, segmentMaskOrSize=3, indexOrOffset=5,
debugName=r5], type=int], INT_TO_BYTE]
3: [VMLoad[storage=VMStorage[type=0, segmentMaskOrSize=3, indexOrOffset=6,
debugName=r6], type=int], INT_TO_BYTE]
4: [VMLoad[storage=VMStorage[type=0, segmentMaskOrSize=3, indexOrOffset=7,
debugName=r7], type=int], INT_TO_BYTE]
5: [VMLoad[storage=VMStorage[type=0, segmentMaskOrSize=3, indexOrOffset=8,
debugName=r8], type=int], INT_TO_BYTE]
6: [VMLoad[storage=VMStorage[type=0, segmentMaskOrSize=3, indexOrOffset=9,
debugName=r9], type=int], INT_TO_BYTE]
7: [VMLoad[storage=VMStorage[type=0, segmentMaskOrSize=3, indexOrOffset=10,
debugName=r10], type=int], INT_TO_BYTE]
8: [VMLoad[storage=VMStorage[type=2, segmentMaskOrSize=8, indexOrOffset=64,
debugName=Stack@64], type=int], INT_TO_BYTE]
Return: [BufferLoad[offset=0, type=long, byteWidth=8],
VMStore[storage=VMStorage[type=0, segmentMaskOrSize=3, indexOrOffset=3,
debugName=r3], type=long]]
}
I have fixed the issue and enabled the test. My code still contained an old
assumption which doesn't hold for the cases I had implemented more recently.
So, this PR is probably complete. I'll rerun tests and wait for 2 reviews.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v16]
On Tue, 14 Mar 2023 22:30:22 GMT, Martin Doerr wrote:
>> Implementation of "Foreign Function & Memory API" for linux on Power (Little
>> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>>
>> This PR does not include code for VaList support because it's supposed to
>> get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736).
>> I've kept the related tests disabled for this platform and throw an
>> exception instead. Note that the ABI doesn't precisely specify variable
>> argument lists. Instead, it refers to `` (2.2.4 Variable Argument
>> Lists).
>>
>> Big Endian support is implemented to some extend, but not complete. E.g.
>> structs with size not divisible by 8 are not passed correctly (see
>> `useABIv2` in CallArranger.java). Big Endian is excluded by selecting
>> `ARCH.equals("ppc64le")` (CABI.java) only.
>>
>> There's another limitation: This PR only accepts structures with size
>> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
>> think arbitrary sizes are not usable on other platforms, either, because
>> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
>> get addressed separately:
>> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>>
>> The ABI has some tricky corner cases related to HFA (Homogeneous Float
>> Aggregate). The same argument may need to get passed in both, a FP reg and a
>> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
>> by the existing tests.
>>
>> I had to make changes to shared code and code for other platforms:
>> 1. Pass type information when creating `VMStorage` objects from `VMReg`.
>> This is needed for the following reasons:
>> - PPC64 ABI requires integer types to get extended to 64 bit (also see
>> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
>> know the type or at least the bit width for that.
>> - Floating point load / store instructions need the correct width to select
>> between the correct IEEE 754 formats. The register representation in single
>> FP registers is always IEEE 754 double precision on PPC64.
>> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
>> loading 4 Bytes yields different values than on Little Endian!
>> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer
>> (with byteSize() == 0) while running TestUpcallScope. Hence, existing size
>> checks don't work (see MemorySegment.java). As a workaround, I'm just
>> skipping the check in this particular case. Please check if this makes sense
>> or if there's a better fix (possibly as separate RFE). Update: This issue is
>> resolved by 2nd commit.
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix storing 32 bit integers into Java frames. Enable TestArrayStructs.
Btw. the new cases in which we use int and short accesses when byteWidth is not
a power of 2 are never unaligned AFAICS. I guess _UNALIGNED is unnecessary in
the JAVA_INT_UNALIGNED and JAVA_SHORT_UNALIGNED. They are always aligned wrt.
to their size.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v17]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Fix Copyright format.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/9173af20..5320f895
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=16
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=15-16
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v16]
On Tue, 14 Mar 2023 22:52:44 GMT, Martin Doerr wrote: >> Martin Doerr has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix storing 32 bit integers into Java frames. Enable TestArrayStructs. > > Btw. the new cases in which we use int and short accesses when byteWidth is > not a power of 2 are never unaligned AFAICS. I guess _UNALIGNED is > unnecessary in the JAVA_INT_UNALIGNED and JAVA_SHORT_UNALIGNED. They are > always aligned wrt. to their size. > @TheRealMDoerr I've approved the PR. I suggest for a second reviewer to try > and get someone who knows the PPC port. Thanks for the review! I guess I should also wait for your 2 PRs to be integrated, so I can merge and adapt. - PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v18]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains 18 commits:
- Merge branch 'master' into PPC64_Panama
- Fix Copyright format.
- Fix storing 32 bit integers into Java frames. Enable TestArrayStructs.
- Allow TestHFA to run on musl. Add Upcalls.
- Introduce ABIv2CallArranger for linux ppc64le.
- Remove LinuxPPC64CallArranger.java because it doesn't contain anything.
- The merge change has messed up some includes in the tests. Revert.
- Remove STRUCT_REFERENCE which was incorrectly taken from aarch64. Pass size
to bufferLoad/Store. Enable TestNested.java.
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Handle HFA corner cases with overlapping registers in Java.
- ... and 8 more: https://git.openjdk.org/jdk/compare/7dbab81d...944742b5
-
Changes: https://git.openjdk.org/jdk/pull/12708/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=17
Stats: 2434 lines in 61 files changed: 2324 ins; 1 del; 109 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v19]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains 19 commits:
- Merge branch 'openjdk:master' into PPC64_Panama
- Merge branch 'master' into PPC64_Panama
- Fix Copyright format.
- Fix storing 32 bit integers into Java frames. Enable TestArrayStructs.
- Allow TestHFA to run on musl. Add Upcalls.
- Introduce ABIv2CallArranger for linux ppc64le.
- Remove LinuxPPC64CallArranger.java because it doesn't contain anything.
- The merge change has messed up some includes in the tests. Revert.
- Remove STRUCT_REFERENCE which was incorrectly taken from aarch64. Pass size
to bufferLoad/Store. Enable TestNested.java.
- Merge remote-tracking branch 'origin' into PPC64_Panama
- ... and 9 more: https://git.openjdk.org/jdk/compare/f6291520...2f2f41be
-
Changes: https://git.openjdk.org/jdk/pull/12708/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=18
Stats: 2434 lines in 61 files changed: 2324 ins; 1 del; 109 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v20]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with two additional
commits since the last revision:
- Adaptation for JDK-8303022.
- Adaptation for JDK-8303684.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/2f2f41be..a21f6cfb
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=19
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=18-19
Stats: 23 lines in 5 files changed: 9 ins; 3 del; 11 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v21]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
> get addressed separately:
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the
AIX/linux separation.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/a21f6cfb..4666aa22
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=20
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=19-20
Stats: 81 lines in 3 files changed: 40 ins; 40 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v20]
On Thu, 16 Mar 2023 14:17:12 GMT, Martin Doerr wrote:
>> Implementation of "Foreign Function & Memory API" for linux on Power (Little
>> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>>
>> This PR does not include code for VaList support because it's supposed to
>> get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736).
>> I've kept the related tests disabled for this platform and throw an
>> exception instead. Note that the ABI doesn't precisely specify variable
>> argument lists. Instead, it refers to `` (2.2.4 Variable Argument
>> Lists).
>>
>> Big Endian support is implemented to some extend, but not complete. E.g.
>> structs with size not divisible by 8 are not passed correctly (see
>> `useABIv2` in CallArranger.java). Big Endian is excluded by selecting
>> `ARCH.equals("ppc64le")` (CABI.java) only.
>>
>> There's another limitation: This PR only accepts structures with size
>> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
>> think arbitrary sizes are not usable on other platforms, either, because
>> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update: Will
>> get addressed separately:
>> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>>
>> The ABI has some tricky corner cases related to HFA (Homogeneous Float
>> Aggregate). The same argument may need to get passed in both, a FP reg and a
>> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
>> by the existing tests.
>>
>> I had to make changes to shared code and code for other platforms:
>> 1. Pass type information when creating `VMStorage` objects from `VMReg`.
>> This is needed for the following reasons:
>> - PPC64 ABI requires integer types to get extended to 64 bit (also see
>> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
>> know the type or at least the bit width for that.
>> - Floating point load / store instructions need the correct width to select
>> between the correct IEEE 754 formats. The register representation in single
>> FP registers is always IEEE 754 double precision on PPC64.
>> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
>> loading 4 Bytes yields different values than on Little Endian!
>> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer
>> (with byteSize() == 0) while running TestUpcallScope. Hence, existing size
>> checks don't work (see MemorySegment.java). As a workaround, I'm just
>> skipping the check in this particular case. Please check if this makes sense
>> or if there's a better fix (possibly as separate RFE). Update: This issue is
>> resolved by 2nd commit.
>
> Martin Doerr has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Adaptation for JDK-8303022.
> - Adaptation for JDK-8303684.
Rebasing + minor cleanup done. Tests are passing.
-
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v9]
On Sat, 8 Apr 2023 18:00:53 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Remove unused static and import of Stabile Would be great if you could support "os.arch = ppc64" for AIX and legacy linux, too. test/jdk/jdk/internal/util/ArchTest.java line 128: > 126: case RISCV64 -> true; > 127: case S390 -> false; > 128: case PPC64 -> true; This is not always true. The PPC64 architecture supports both endianness versions. AIX and legacy linux is Big Endian while recent linux is little Endian (determined by platform name "os.arch = ppc64le" instead of "os.arch = ppc64"). Querying the endianness is also possible, of course. - Changes requested by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13357#pullrequestreview-1378905956 PR Review Comment: https://git.openjdk.org/jdk/pull/13357#discussion_r1162600674
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v9]
On Tue, 11 Apr 2023 10:26:02 GMT, ExE Boss wrote: >> test/jdk/jdk/internal/util/ArchTest.java line 128: >> >>> 126: case RISCV64 -> true; >>> 127: case S390 -> false; >>> 128: case PPC64 -> true; >> >> This is not always true. The PPC64 architecture supports both endianness >> versions. AIX and legacy linux is Big Endian while recent linux is little >> Endian (determined by platform name "os.arch = ppc64le" instead of "os.arch >> = ppc64"). Querying the endianness is also possible, of course. > > This should (probably) be correct: > Suggestion: > > case PPC64 -> !OperatingSystem.isAix() && > Architecture.isLittleEndian(); Looks correct, but makes the test pointless for any linux on PPC64. - PR Review Comment: https://git.openjdk.org/jdk/pull/13357#discussion_r1162628828
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v10]
On Tue, 11 Apr 2023 17:58:54 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Modified test to check Architecture is64bits() and isLittleEndian() > against Unsafe respective values. > Relocated code mapping OS name and arch name from PlatformProps to > OperatingSystem and Architecture. Kept the mapping of names > in the template close to where the values are filled in by the build. test/jdk/jdk/internal/util/ArchTest.java line 74: > 72: case "riscv64" -> RISCV64; // unverified > 73: case "s390x", "s390" -> S390; // unverified > 74: case "ppc64le" -> PPC64; // unverified I think "ppc64" should also get mapped to "PPC64". - PR Review Comment: https://git.openjdk.org/jdk/pull/13357#discussion_r1163161387
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v10]
On Tue, 11 Apr 2023 17:58:54 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Modified test to check Architecture is64bits() and isLittleEndian() > against Unsafe respective values. > Relocated code mapping OS name and arch name from PlatformProps to > OperatingSystem and Architecture. Kept the mapping of names > in the template close to where the values are filled in by the build. Another remark: Old JDK on s390 used "os.arch = zArch_64", current one "os.arch = s390x". @offamitkumar: You probably want to take a look. - PR Comment: https://git.openjdk.org/jdk/pull/13357#issuecomment-1503861585
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v11]
On Tue, 11 Apr 2023 18:35:56 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Add ppc64 as mapping to PPC64 Architecture Thanks for the updates! PPC64 looks basically good except of a upper / lower case issue in the test: STARTEDArchTest::nameVsCurrent 'nameVsCurrent()' org.opentest4j.AssertionFailedError: mismatch in Architecture.current vs ppc64 ==> expected: but was: ... FAILED ArchTest::nameVsCurrent 'nameVsCurrent()' JavaTest Message: JUnit Platform Failure(s): 1 - PR Comment: https://git.openjdk.org/jdk/pull/13357#issuecomment-1504064549
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v12]
On Tue, 11 Apr 2023 21:09:43 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Correct mapping and test of ppc64 Works on PPC64 Big Endian, now. However, little Endian fails: org.opentest4j.AssertionFailedError: Mismatch PPC64 == PPC64 vs isPPC64 ==> expected: but was: at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55) at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62) at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182) at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1152) at ArchTest.isArch(ArchTest.java:99) ... System property os.arch: "ppc64le", Architecture.current(): "PPC64" - PR Comment: https://git.openjdk.org/jdk/pull/13357#issuecomment-1504925182
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v13]
On Wed, 12 Apr 2023 17:31:49 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed isPPC64(). > Consolidated switch cases in ArchTest. > Moved mapping of build TARGET_OS and TARGET_CPU to the build > to avoid multiple mappings in more than one place. Thanks! PPC64 parts look good and the test has passed on both endianness versions. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13357#pullrequestreview-1381866371
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains 24 commits:
- Adaptation for JDK-8305668
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the
AIX/linux separation.
- Adaptation for JDK-8303022.
- Adaptation for JDK-8303684.
- Merge branch 'openjdk:master' into PPC64_Panama
- Merge branch 'master' into PPC64_Panama
- Fix Copyright format.
- Fix storing 32 bit integers into Java frames. Enable TestArrayStructs.
- Allow TestHFA to run on musl. Add Upcalls.
- ... and 14 more: https://git.openjdk.org/jdk/compare/3bba8995...725732a0
-
Changes: https://git.openjdk.org/jdk/pull/12708/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=21
Stats: 2440 lines in 62 files changed: 2330 ins; 1 del; 109 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8306008: Several Vector API tests fail for client VM after JDK-8304450
On Tue, 18 Apr 2023 12:21:11 GMT, Quan Anh Mai wrote: > Hi, > > Please review this patch which fixes the errors on machines where > TypeMaxVector has a length of 64 bits. I take an extra cautious approach and > fall back to putting elements one by one if the length is an unexpected value. > > All jdk/incubator/vector tests passed with MaxVectorSize=8, which fails > without this patch. > > Thanks a lot. Thanks for fixing it! This PR fixes the issues in Vector64ConversionTests on PPC64. - PR Comment: https://git.openjdk.org/jdk/pull/13508#issuecomment-1513071561
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v15]
On Mon, 17 Apr 2023 20:59:06 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 17 commits: > > - Merge branch 'master' into 8304915-arch-enum > - ArchTest on Debian RISC-V 64 confirmed by reviewer > - Fixed isPPC64(). >Consolidated switch cases in ArchTest. >Moved mapping of build TARGET_OS and TARGET_CPU to the build >to avoid multiple mappings in more than one place. > - Correct mapping and test of ppc64 > - Add ppc64 as mapping to PPC64 Architecture > - Modified test to check Architecture is64bits() and isLittleEndian() >against Unsafe respective values. >Relocated code mapping OS name and arch name from PlatformProps to >OperatingSystem and Architecture. Kept the mapping of names >in the template close to where the values are filled in by the build. > - Remove unused static and import of Stabile > - Rename OperatingSystemProps to PlatformProps. >Refactor OperatingSystem initialization to use strings instead of integers. > - Revised mapping mechanism of build target architecture names to enum > values. >Unrecognized values from the build are mapped to enum value "OTHER". >Renamed PPC64LE to PPC64 to reflect only the architecture, not the > endianness. >Added an `isLittleEndian` method to return the endianness (not currently > used anywhere) > - Revert changes to jdk.accessibility AccessBridge > - ... and 7 more: https://git.openjdk.org/jdk/compare/8858d543...99a93b7e test/jdk/jdk/internal/util/ArchTest.java line 71: > 69: case "aarch64" -> AARCH64; > 70: case "riscv64" -> RISCV64; > 71: case "s390x", "s390" -> S390; // unverified This was also verified according to comments. Right, @offamitkumar? - PR Review Comment: https://git.openjdk.org/jdk/pull/13357#discussion_r1171335657
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v17]
On Fri, 21 Apr 2023 20:26:12 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Correct comment on isPPC64() and refer to isLittleEndian() instead of > mentioning PPC64LE Thanks for the comment updates! - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13357#pullrequestreview-1397600716
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v23]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains 27 commits:
- Enable remaining foreign tests.
- Adaptations for JDK-8304265.
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Adaptation for JDK-8305668
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the
AIX/linux separation.
- Adaptation for JDK-8303022.
- Adaptation for JDK-8303684.
- Merge branch 'openjdk:master' into PPC64_Panama
- Merge branch 'master' into PPC64_Panama
- ... and 17 more: https://git.openjdk.org/jdk/compare/1be80a44...84e9dd2f
-
Changes: https://git.openjdk.org/jdk/pull/12708/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=22
Stats: 2556 lines in 70 files changed: 2393 ins; 6 del; 157 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v24]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Revert unintended formatting changes. Fix comment.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/84e9dd2f..7b85fca9
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=23
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=22-23
Stats: 189 lines in 6 files changed: 5 ins; 51 del; 133 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v24]
On Thu, 27 Apr 2023 12:54:55 GMT, Martin Doerr wrote:
>> Implementation of "Foreign Function & Memory API" for linux on Power (Little
>> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>>
>> This PR does not include code for VaList support because it's supposed to
>> get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736).
>> I've kept the related tests disabled for this platform and throw an
>> exception instead. Note that the ABI doesn't precisely specify variable
>> argument lists. Instead, it refers to `` (2.2.4 Variable Argument
>> Lists).
>>
>> Big Endian support is implemented to some extend, but not complete. E.g.
>> structs with size not divisible by 8 are not passed correctly (see
>> `useABIv2` in CallArranger.java). Big Endian is excluded by selecting
>> `ARCH.equals("ppc64le")` (CABI.java) only.
>>
>> There's another limitation: This PR only accepts structures with size
>> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
>> think arbitrary sizes are not usable on other platforms, either, because
>> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
>> Resolved after merging of
>> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>>
>> The ABI has some tricky corner cases related to HFA (Homogeneous Float
>> Aggregate). The same argument may need to get passed in both, a FP reg and a
>> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
>> by the existing tests.
>>
>> I had to make changes to shared code and code for other platforms:
>> 1. Pass type information when creating `VMStorage` objects from `VMReg`.
>> This is needed for the following reasons:
>> - PPC64 ABI requires integer types to get extended to 64 bit (also see
>> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
>> know the type or at least the bit width for that.
>> - Floating point load / store instructions need the correct width to select
>> between the correct IEEE 754 formats. The register representation in single
>> FP registers is always IEEE 754 double precision on PPC64.
>> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
>> loading 4 Bytes yields different values than on Little Endian!
>> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer
>> (with byteSize() == 0) while running TestUpcallScope. Hence, existing size
>> checks don't work (see MemorySegment.java). As a workaround, I'm just
>> skipping the check in this particular case. Please check if this makes sense
>> or if there's a better fix (possibly as separate RFE). Update: This issue is
>> resolved by 2nd commit.
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Revert unintended formatting changes. Fix comment.
Adapted for JDK21, now. All tests have passed. My IDE had changed the
formatting which is reverted, now. (I've kept the minor formatting changes in
TestDontRelease.java because it looks better.)
-
PR Comment: https://git.openjdk.org/jdk/pull/12708#issuecomment-1525655158
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v25]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains 30 commits:
- Adaptation for JDK-8303002.
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Revert unintended formatting changes. Fix comment.
- Enable remaining foreign tests.
- Adaptations for JDK-8304265.
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Adaptation for JDK-8305668
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the
AIX/linux separation.
- Adaptation for JDK-8303022.
- ... and 20 more: https://git.openjdk.org/jdk/compare/860bf9b3...f5e22be0
-
Changes: https://git.openjdk.org/jdk/pull/12708/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=24
Stats: 2465 lines in 69 files changed: 2348 ins; 1 del; 116 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v26]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains 32 commits:
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Prepare for JDK-8304888. (Revert test changes.)
- Adaptation for JDK-8303002.
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Revert unintended formatting changes. Fix comment.
- Enable remaining foreign tests.
- Adaptations for JDK-8304265.
- Merge remote-tracking branch 'origin' into PPC64_Panama
- Adaptation for JDK-8305668
- Merge remote-tracking branch 'origin' into PPC64_Panama
- ... and 22 more: https://git.openjdk.org/jdk/compare/a8bf2acb...e4ddbda0
-
Changes: https://git.openjdk.org/jdk/pull/12708/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=25
Stats: 2395 lines in 27 files changed: 2348 ins; 0 del; 47 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v25]
On Tue, 2 May 2023 14:01:57 GMT, Jorn Vernee wrote: > On another note, how are you coming along with finding another reviewer? I > (still) think it would be good to get someone that is familiar with PPC > (particularly the ABI) as a second reviewer. Second Review is in progress. I have merged your recent changes and all tests are passing. Your updates made my PR much smaller :-) Do you have for more changes to wait for or would you prefer to have this PR integrated soon? Off topic: I have read parts of the Big Endian ABI and we will need a solution for "An aggregate or union smaller than one doubleword in size is padded so that it appears in the least significant bits of the doubleword. All others are padded, if necessary, at their tail." The tail padding seems to be tricky for Big Endian as we currently access the wrong bytes. I think it could be solved by dirty hacks (shifting) in the backend, but that doesn't sound like a good solution. Do you have a good idea for that? Maybe shift or pad in Java? - PR Comment: https://git.openjdk.org/jdk/pull/12708#issuecomment-1531594500
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v26]
On Tue, 2 May 2023 14:24:39 GMT, Martin Doerr wrote:
>> Implementation of "Foreign Function & Memory API" for linux on Power (Little
>> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>>
>> This PR does not include code for VaList support because it's supposed to
>> get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736).
>> I've kept the related tests disabled for this platform and throw an
>> exception instead. Note that the ABI doesn't precisely specify variable
>> argument lists. Instead, it refers to `` (2.2.4 Variable Argument
>> Lists).
>>
>> Big Endian support is implemented to some extend, but not complete. E.g.
>> structs with size not divisible by 8 are not passed correctly (see
>> `useABIv2` in CallArranger.java). Big Endian is excluded by selecting
>> `ARCH.equals("ppc64le")` (CABI.java) only.
>>
>> There's another limitation: This PR only accepts structures with size
>> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
>> think arbitrary sizes are not usable on other platforms, either, because
>> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
>> Resolved after merging of
>> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>>
>> The ABI has some tricky corner cases related to HFA (Homogeneous Float
>> Aggregate). The same argument may need to get passed in both, a FP reg and a
>> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
>> by the existing tests.
>>
>> I had to make changes to shared code and code for other platforms:
>> 1. Pass type information when creating `VMStorage` objects from `VMReg`.
>> This is needed for the following reasons:
>> - PPC64 ABI requires integer types to get extended to 64 bit (also see
>> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
>> know the type or at least the bit width for that.
>> - Floating point load / store instructions need the correct width to select
>> between the correct IEEE 754 formats. The register representation in single
>> FP registers is always IEEE 754 double precision on PPC64.
>> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
>> loading 4 Bytes yields different values than on Little Endian!
>> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer
>> (with byteSize() == 0) while running TestUpcallScope. Hence, existing size
>> checks don't work (see MemorySegment.java). As a workaround, I'm just
>> skipping the check in this particular case. Please check if this makes sense
>> or if there's a better fix (possibly as separate RFE). Update: This issue is
>> resolved by 2nd commit.
>
> Martin Doerr has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 32 commits:
>
> - Merge remote-tracking branch 'origin' into PPC64_Panama
> - Prepare for JDK-8304888. (Revert test changes.)
> - Adaptation for JDK-8303002.
> - Merge remote-tracking branch 'origin' into PPC64_Panama
> - Revert unintended formatting changes. Fix comment.
> - Enable remaining foreign tests.
> - Adaptations for JDK-8304265.
> - Merge remote-tracking branch 'origin' into PPC64_Panama
> - Adaptation for JDK-8305668
> - Merge remote-tracking branch 'origin' into PPC64_Panama
> - ... and 22 more: https://git.openjdk.org/jdk/compare/a8bf2acb...e4ddbda0
> In general the assumption of the linker is that any layouts it is given are
> correct for the given platform/ABI. i.e. it is up to the user to specify the
> correct padding (and this is where jextract can help out a lot). We do also
> check that layouts are 'canonical' now (see #13164 & [1]). I think this
> already guarantees that the necessary trailing padding is present (constraint
> 3)? Did you see the discussion at [2] ? I think we already have Big Endian
> covered?
I had seen that discussion. It appears to work for the TestMiniStruct I had
uploaded (passed a structure consisting of 3 bytes). However, I'm getting
failures when passing larger structs which have a size >8, but not divisible by
8. E.g. 17 tests of TestDowncallScope are failing. The guarantee was not hit.
-
PR Comment: https://git.openjdk.org/jdk/pull/12708#issuecomment-1531709856
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v27]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Add test case for passing a double value in a GP register. Use better
instructions for moving between FP and GP reg. Improve comments.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/e4ddbda0..754a19a0
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=26
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=25-26
Stats: 110 lines in 4 files changed: 88 ins; 2 del; 20 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v28]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
libTestHFA: Add explicit type conversion to avoid build warning.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/754a19a0..74586ab8
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=27
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=26-27
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v29]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Replace NULL by nullptr.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/74586ab8..93060258
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=28
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=27-28
Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v21]
On Mon, 27 Mar 2023 16:54:31 GMT, Richard Reingruber wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the
>> AIX/linux separation.
>
> src/hotspot/cpu/ppc/downcallLinker_ppc.cpp line 185:
>
>> 183:
>> 184: allocated_frame_size = align_up(allocated_frame_size,
>> StackAlignmentInBytes);
>> 185: _frame_size_slots = allocated_frame_size >> LogBytesPerInt;
>
> `VMRegImpl::stack_slot_size` could be used when converting from size in bytes
> to size in slots.
Yes, I think this would be better readable. But the following code should also
be adapted, then:
int framesize() const {
return (_frame_size_slots >> (LogBytesPerWord - LogBytesPerInt));
}
Maybe it makes sense to do some cleanup for all platforms.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189741783
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 26 Apr 2023 14:32:59 GMT, Richard Reingruber wrote: >> Martin Doerr has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 24 commits: >> >> - Adaptation for JDK-8305668 >> - Merge remote-tracking branch 'origin' into PPC64_Panama >> - Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the >> AIX/linux separation. >> - Adaptation for JDK-8303022. >> - Adaptation for JDK-8303684. >> - Merge branch 'openjdk:master' into PPC64_Panama >> - Merge branch 'master' into PPC64_Panama >> - Fix Copyright format. >> - Fix storing 32 bit integers into Java frames. Enable TestArrayStructs. >> - Allow TestHFA to run on musl. Add Upcalls. >> - ... and 14 more: https://git.openjdk.org/jdk/compare/3bba8995...725732a0 > > src/hotspot/cpu/ppc/frame_ppc.cpp line 219: > >> 217: UpcallStub* blob = _cb->as_upcall_stub(); >> 218: JavaFrameAnchor* jfa = blob->jfa_for_frame(*this); >> 219: return jfa->last_Java_sp() == NULL; > > Suggestion: > > return jfa->last_Java_sp() == nullptr; > > I'd suggest to do the same for all occurrences in the patch. Good catch! I've replaced them. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189742225
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 26 Apr 2023 14:41:51 GMT, Richard Reingruber wrote: >> Martin Doerr has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 24 commits: >> >> - Adaptation for JDK-8305668 >> - Merge remote-tracking branch 'origin' into PPC64_Panama >> - Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the >> AIX/linux separation. >> - Adaptation for JDK-8303022. >> - Adaptation for JDK-8303684. >> - Merge branch 'openjdk:master' into PPC64_Panama >> - Merge branch 'master' into PPC64_Panama >> - Fix Copyright format. >> - Fix storing 32 bit integers into Java frames. Enable TestArrayStructs. >> - Allow TestHFA to run on musl. Add Upcalls. >> - ... and 14 more: https://git.openjdk.org/jdk/compare/3bba8995...725732a0 > > src/hotspot/cpu/ppc/methodHandles_ppc.cpp line 316: > >> 314: // Load the invoker, as NEP -> .invoker >> 315: __ verify_oop(nep_reg); >> 316: __ ld(temp_target, >> jdk_internal_foreign_abi_NativeEntryPoint::downcall_stub_address_offset_in_bytes(), >> nep_reg); > > Other platforms use `access_load_at`. Interesting. I have no idea why. It does the same but with a more complicated API. I just noticed that other platforms use `NONZERO`. I think I should at least add that. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189752212
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v24]
On Thu, 27 Apr 2023 16:19:46 GMT, Richard Reingruber wrote: >> Martin Doerr has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert unintended formatting changes. Fix comment. > > src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 202: > >> 200: >> 201: MacroAssembler* _masm = new MacroAssembler(&buffer); >> 202: address start = __ function_entry(); // called by C > > If `!defined(ABI_ELFv2)` a function descriptor will be emitted here. It will > be initialized with `friend_toc` and `friend_env`. But that's not correct for > external callers, is it? If so, wouldn't an `Unimplemented()` be better than > obscure crashes? No, this code is correct and tested (I have a partially working Big Endian patch). `toc` and `env` are loaded by the external caller (C code), but not used by the stub. So, we don't need to initialize them to any specific values. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189755698
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v24]
On Fri, 28 Apr 2023 13:18:27 GMT, Richard Reingruber wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Revert unintended formatting changes. Fix comment.
>
> src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 236:
>
>> 234: __ block_comment("{ receiver ");
>> 235: __ load_const_optimized(R3_ARG1, (intptr_t)receiver, R0);
>> 236: __ resolve_jobject(R3_ARG1, tmp, R31,
>> MacroAssembler::PRESERVATION_FRAME_LR_GP_FP_REGS); // kills R31
>
> As a simplification the receiver could be resolved in
> `UpcallLinker::on_entry` and returned in `JavaThread::_vm_result`.
This sounds like a nice enhancement proposal for all platforms. The register
spilling code in `resolve_jobject` can get lengthy dependent on the selected
GC. Doing it in the C code (which we call anyway above) would make the upcall
stubs smaller.
@JornVernee: What do you think about this idea?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189763910
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v28]
On Tue, 9 May 2023 15:48:52 GMT, Richard Reingruber wrote: >> Martin Doerr has updated the pull request incrementally with one additional >> commit since the last revision: >> >> libTestHFA: Add explicit type conversion to avoid build warning. > > src/hotspot/cpu/ppc/vmstorage_ppc.hpp line 81: > >> 79: case T_BYTE : >> 80: case T_SHORT : >> 81: case T_INT: segment_mask = REG32_MASK; break; > > I wonder why the segment_mask depends on `bt` on ppc? The usage of the `segment_mask` can be defined for each platform. I'm using it to encode the information if a value on the Java side uses a 32 or 64 bit slot. In case of 32 bit values, the C side requires all 64 register bits to get defined values (ints get sign extended, floats get converted to double-precision format). - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189768204
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 13:22:48 GMT, Richard Reingruber wrote: >>> It does the same but with a more complicated API. >> >> AFAIK It depends on the GC that's being used. `access_load_at` will make >> sure the right GC barriers are inserted (mostly for concurrent GCs). > > As I see it, the access API is an abstraction to be used instead of raw > loads. It hides details. See for instance `TemplateTable::getfield_or_static` > on x86 where it is also used. PPC lags behind in making use of the access API. > With a fancy new GC the oop in nep_reg could be stale, requiring some > processing which would be taken care of by using the access API. GC barriers are used when loading or storing an oop. No GC we currently have (not even the generational ones) use barriers for loading a plain address from an oop. The PPC64 implementation of the BarrierSetAssembler currently has `Unimplemented()` for non-oop types and all GCs are implemented. Maybe it was intended for some future GC or other feature which has not yet reached the official repo. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189908142
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 13:33:02 GMT, Richard Reingruber wrote: >> GC barriers are used when loading or storing an oop. No GC we currently have >> (not even the generational ones) use barriers for loading a plain address >> from an oop. The PPC64 implementation of the BarrierSetAssembler currently >> has `Unimplemented()` for non-oop types and all GCs are implemented. >> Maybe it was intended for some future GC or other feature which has not yet >> reached the official repo. > > You are reasoning about implementation details. By using the provided > abstraction you and other maintainers (who might be unfamiliar with them) > would not have to do that. Also the assumptions you make introduce a hidden > dependency. I just figured it out. It was introduced by https://bugs.openjdk.org/browse/JDK-8203172 (on aarch64) which mentions Shenandoah and future GCs. However, the Shenandoah comment says "non-reference load, no additional barrier is needed" and it doesn't use barriers in such a case. So, for the time being, I'll keep the normal load (because `access_load_at` is not ready for non-oop types). But I should add the `NONZERO` check. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189934352
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v30]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Add NONZERO check for downcall_stub_address_offset_in_bytes().
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/93060258..edcdefba
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=29
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12708&range=28-29
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 14:26:54 GMT, Jorn Vernee wrote: >> FWIW, since Shenandoah changed their load barriers we have been cleaning >> away the usages of the Access API for loads and stores to primitive values. >> There's no such support in the C++ Runtime code. > > Ok, since this is loading a `long` (which represents an address that points > into the code cache) I think we're fine without using the access API then? Correct. The code had been written for the previous version of Shenandoah (1.0). No current GC uses barriers for non-oop types and the C++ Runtime doesn't support it any more as Stefan pointed out. It is still possible to use the access API on other platforms, but it does nothing more than a plain load/store for non-oop types. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1190447075
RFR: 8308246: PPC64le build broken after JDK-8304913
PPC64le needs to get recognized as PPC64. Otherwise, jlink doesn't recognize the platform and throws a PluginException: ModuleTarget is malformed: No enum constant jdk.internal.util.Architecture.PPC64LE. - Commit messages: - 8308246: PPC64le build broken after JDK-8304913 Changes: https://git.openjdk.org/jdk/pull/14027/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14027&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8308246 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/14027.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14027/head:pull/14027 PR: https://git.openjdk.org/jdk/pull/14027
Re: RFR: 8308246: PPC64le build broken after JDK-8304913
On Wed, 17 May 2023 09:00:01 GMT, Alan Bateman wrote:
>> PPC64le needs to get recognized as PPC64. Otherwise, jlink doesn't recognize
>> the platform and throws a PluginException: ModuleTarget is malformed: No
>> enum constant jdk.internal.util.Architecture.PPC64LE.
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Platform.java line 55:
>
>> 53: // Alias architecture "amd64" to "X64" and "ppc64le" to "ppc64"
>> 54: archName = archName.replace("amd64", "X64")
>> 55:.replace("ppc64le", "ppc64");
>
> Is linux-ppc64 big endian? If so, a module with a ModuleTarget value of
> linux-ppc64le can't be linked with a module that has ModuleTarget value of
> linux-ppc64. I'm not say the aliasing here is wrong but I am curious if
> attempts to link will fail.
Yes, linux-ppc64 is big endian. The 2 flavors are completely incompatible.
linux-ppc64le is basically a new platform. Endianness should actually get
checked in addition to the archName.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14027#discussion_r1196183985
Re: RFR: 8308246: PPC64le build broken after JDK-8304913
On Wed, 17 May 2023 08:48:53 GMT, Aleksey Shipilev wrote:
>> PPC64le needs to get recognized as PPC64. Otherwise, jlink doesn't recognize
>> the platform and throws a PluginException: ModuleTarget is malformed: No
>> enum constant jdk.internal.util.Architecture.PPC64LE.
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Platform.java line 55:
>
>> 53: // Alias architecture "amd64" to "X64" and "ppc64le" to "ppc64"
>> 54: archName = archName.replace("amd64", "X64")
>> 55:.replace("ppc64le", "ppc64");
>
> I'd prefer it to be:
>
> Suggestion:
>
> // Alias architecture names, if needed
> archName = archName.replace("amd64", "X64");
> archName = archName.replace("ppc64le", "PPC64");
>
>
> So that other arches would add here more naturally.
>
> (Also convert to upper-case right away, to make the subsequent
> `archName.toUpperCase` no-op).
Makes sense. Thanks for the review!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14027#discussion_r1196188653
