On Mon, 9 Nov 2026 11:57:20 GMT, Martin Doerr <[email protected]> wrote:

>> Jorn Vernee has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 49 commits:
>> 
>>  - AIX test fix
>>  - Merge branch 'master' into SignInfo
>>  - Update copyright years
>>  - Try to avoid compiler warning
>>  - Don't use list
>>  - Fix canonical layouts + add test
>>  - Fix @since errors
>>  - re-add requires tag
>>  - Rework tests
>>  - Sign -> signedness paramter name
>>  - ... and 39 more: https://git.openjdk.org/jdk/compare/ce0c58ba...3a1f1c7f
>
> src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java line 
> 394:
> 
>> 392: 
>> 393:     public static boolean isSigned(MemoryLayout layout) {
>> 394:         return !(layout instanceof OfIntegral integral) || 
>> integral.isSigned();
> 
> May be a bit confusing that non-integral MemoryLayouts are signed, but ok.

I agree. I've just removed this method in favor of casting directly to 
`OfIntegeral` at the use site (making it clear there's an assumption  being 
made about the layout type).

> test/jdk/java/foreign/normalize/TestZeroExtendInt.java line 30:
> 
>> 28:  * @requires (os.name == "Mac OS X")
>> 29:  *           | (os.name == "Linux"
>> 30:  *               & (os.arch == "amd64" | os.arch == "ppc64" | os.arch == 
>> "ppc64le" | os.arch == "riscv64"))
> 
> Why are AIX and linux s390 excluded, but linux on ppc64 included? The test 
> passes on all of these platforms.

Good catch, I forgot to include those after writing the initial test.

> test/jdk/java/foreign/normalize/TestZeroExtendLong.java line 28:
> 
>> 26:  * @requires jdk.foreign.linker != "FALLBACK"
>> 27:  * @library ../
>> 28:  * @requires os.arch == "ppc64" | os.arch == "ppc64le" | os.arch == 
>> "s390x"
> 
> This test also passes on linux x86. Not sure if we should run it.

This seems to be a side effect of the CPU clearing the upper 32 bits of the 
argument register when we pass something in the lower 32 bits only, which we do 
for types smaller than long. Since this is not guaranteed by the ABI, but a 
side-effect of the implementation, I'd rather not rely on it too much. It's 
also not the part of the implementation this test is designed to test.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/33417#discussion_r4526720333
PR Review Comment: https://git.openjdk.org/jdk/pull/33417#discussion_r4526709551
PR Review Comment: https://git.openjdk.org/jdk/pull/33417#discussion_r4526705526

Reply via email to