On Wed, 28 Oct 2026 18:08:39 GMT, Jorn Vernee <[email protected]> wrote:

>> Currently we do not directly support unsigned types in the Linker. The 
>> javadoc has this to say about it:
>> 
>>>  A native linker typically does not provide canonical layouts for C's 
>>> unsigned integral
>>  types. Instead, they are modeled using the canonical layouts associated 
>> with their
>>  corresponding signed integral types. For instance, the C type `unsigned 
>> long`
>>  maps to the canonical layout for `code long`.
>> 
>> This was done under the earlier assumption that ABIs treat signed and 
>> unsigned types the same. However, it turns out that we missed certain cases 
>> where this does matter for some ABIs. More concretely:
>> 
>> - Mac x64 and AArch64 ABIs require integral types smaller than `int`  to be 
>> widened to `int` before being passed to a function 
>> [1](https://developer.apple.com/documentation/xcode/writing-64-bit-intel-code-for-apple-platforms),
>>  
>> [2](https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms).
>>  Clang even extends this behavior to Linux x64 (although this appears to be 
>> a bug) 
>> [3](https://mail.openjdk.org/archives/list/[email protected]/message/RJNMKNUXN7R5PSJETZKX3PMBJ7P6GYJG/).
>> - RISCV requires arguments smaller than XLEN to be extended to 32 bits 
>> according to their sign, and then sign extended to XLEN 
>> [4](https://docs.riscv.org/reference/abi/v1.0/riscv-cc-procedure-calling-convention.html).
>> - PowerPC requires small integer types (char, short, int, long, enum) to be 
>> extended to long 
>> [5](https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html#PARAM-PASS).
>> 
>> In order to address these use cases we need to attach a bit of extra 
>> information to a linkage request that indicates the signedness of certain 
>> arguments. For this patch, we've chosen to attach that information directly 
>> to the argument layouts through a newly introduced `boolean isSigned()` 
>> method on a new `ValueLayout` subclass `OfIntegral` that is implemented by 
>> `OfByte`, `OfShort`, `OfInt`, and `OfLong`.
>> 
>> This information is potentially used by the linker to implement the correct 
>> extension. In cases where it doesn't affect the way functions are linker the 
>> information is erased in order to provide more efficient caching, since 
>> signed and unsigned types can share the same downcall handle in that case.
>> 
>> Testing: `jdk_foreign` test suite on Windows/Linux/Mac-x64, 
>> Mac/Linux-AArch64, Linux-x64 using the fallback linker.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](http...
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Rename withSign in the tests as well

javadoc for JAVA_XYZ constants should perhaps be enhanced to say what the sign 
is

src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 131:

> 129:      * can be either signed or unsigned. The signedness of an integral
> 130:      * value layout has no effect on the memory access operations
> 131:      * provided by this package. However, signedness is relevant when

`provided by this package` sounds odd. Maybe just "on memory access operation 
involving that integral layout" (maybe add a link on accessing memory segment 
javadoc section?).

src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 132:

> 130:      * value layout has no effect on the memory access operations
> 131:      * provided by this package. However, signedness is relevant when
> 132:      * values represented by integral layouts are widened. For example,

instead of "represented" I think "associated with" works better

src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 136:

> 134:      * determines whether the value should be sign extended as specified
> 135:      * by {@jls 5.1.2}, or zero extended as specified by the
> 136:      * {@link Integer#toUnsignedLong(int)} method. A user who is 
> interested

Suggestion:

     * {@link Integer#toUnsignedLong(int)} method. Users interested

src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 137:

> 135:      * by {@jls 5.1.2}, or zero extended as specified by the
> 136:      * {@link Integer#toUnsignedLong(int)} method. A user who is 
> interested
> 137:      * in widening values represented by integral value layouts may wish

Again -- "represented"

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

PR Review: https://git.openjdk.org/jdk/pull/33417#pullrequestreview-5688121977
PR Review Comment: https://git.openjdk.org/jdk/pull/33417#discussion_r4404798059
PR Review Comment: https://git.openjdk.org/jdk/pull/33417#discussion_r4404801988
PR Review Comment: https://git.openjdk.org/jdk/pull/33417#discussion_r4404811444
PR Review Comment: https://git.openjdk.org/jdk/pull/33417#discussion_r4404816466

Reply via email to