Re: RFR: 8308293: A linker should expose the layouts it supports [v4]
On Mon, 22 May 2023 09:34:53 GMT, Maurizio Cimadamore
wrote:
>> This patch adds an instance method on `Linker`, namely
>> `Linker::canonicalLayouts` which returns all the layouts known by the linker
>> as implementing some ABI type. For instance, if I call this on my machine
>> (Linux/x64) I get this:
>>
>>
>> jshell> import java.lang.foreign.*;
>>
>> jshell> Linker.nativeLinker().canonicalLayouts()
>> $2 ==> {char16_t=c16, int8_t=b8, long=j64, size_t=j64, bool=z8, int=i32,
>> long long=j64, int64_t=j64, void*=a64, float=f32, char=b8, int16_t=s16,
>> int32_t=i32, short=s16, double=d64}
>>
>>
>> This can be useful to discover the ABI types supported by a linker
>> implementation, as well as for, in the future, add support for more exotic
>> (and platform-dependent) linker types, such as `long double` or `complex
>> long`.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Address further review comments
Updated javadoc and specdiffs (v3):
javadoc:
https://cr.openjdk.org/~mcimadamore/jdk/8308293/v3/javadoc/java.base/java/lang/foreign/Linker.html
specdiff:
https://cr.openjdk.org/~mcimadamore/jdk/8308293/v3/specdiff_out/overview-summary.html
-
PR Comment: https://git.openjdk.org/jdk/pull/14037#issuecomment-1560749402
Re: RFR: 8308293: A linker should expose the layouts it supports [v4]
On Tue, 23 May 2023 20:32:10 GMT, Maurizio Cimadamore wrote: > Effectively, what you suggest amount at saying: we do have a JAVA_CHAR > layout, which is mostly there for Java interop. But a native linker (which > only cares about native interop) doesn't really care much about that. Does > that sound good? Yes. - PR Comment: https://git.openjdk.org/jdk/pull/14037#issuecomment-1560241481
Re: RFR: 8308293: A linker should expose the layouts it supports [v4]
On Tue, 23 May 2023 20:32:10 GMT, Maurizio Cimadamore wrote: > Arguably C `unsigned short` could map to carriers Java `short` or Java > `char`, but i am inclined to say the user should cast between Java `short` to > `char` in such cases. (Another advantage of this is that, should we get proper unsigned carriers from Valhalla one day, native linkers could be updated to support those en masse - not just for `short`) - PR Comment: https://git.openjdk.org/jdk/pull/14037#issuecomment-1560118602
Re: RFR: 8308293: A linker should expose the layouts it supports [v4]
On Mon, 22 May 2023 09:34:53 GMT, Maurizio Cimadamore
wrote:
>> This patch adds an instance method on `Linker`, namely
>> `Linker::canonicalLayouts` which returns all the layouts known by the linker
>> as implementing some ABI type. For instance, if I call this on my machine
>> (Linux/x64) I get this:
>>
>>
>> jshell> import java.lang.foreign.*;
>>
>> jshell> Linker.nativeLinker().canonicalLayouts()
>> $2 ==> {char16_t=c16, int8_t=b8, long=j64, size_t=j64, bool=z8, int=i32,
>> long long=j64, int64_t=j64, void*=a64, float=f32, char=b8, int16_t=s16,
>> int32_t=i32, short=s16, double=d64}
>>
>>
>> This can be useful to discover the ABI types supported by a linker
>> implementation, as well as for, in the future, add support for more exotic
>> (and platform-dependent) linker types, such as `long double` or `complex
>> long`.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Address further review comments
> On further reflection i think mapping C `unsigned short` only to Java `char`
> is a misleading. Although Java `char` is an integral type is not really
> intended to be used generally as an unsigned 16-bit integer, so arguably Java
> `char` is not as useful a carrier type for native interoperation as Java
> `short` might be even though it is signed. Thus i am inclined to remove that
> mapping.
>
> What if we say something to the effect of:
>
> > unless explicitly declared in the canonical layouts C's unsigned integral
> > types are mapped to the layouts associated with the required C's signed
> > integral types of the same bit sizes.
>
> ?
>
> Arguably C `unsigned short` could map to carriers Java `short` or Java
> `char`, but i am inclined to say the user should cast between Java `short` to
> `char` in such cases.
>
> FWIW i checked what the FFM API and jextract does today and it maps unsigned
> C types to signed Java types.
> On further reflection i think mapping C `unsigned short` only to Java `char`
> is a misleading. Although Java `char` is an integral type is not really
> intended to be used generally as an unsigned 16-bit integer, so arguably Java
> `char` is not as useful a carrier type for native interoperation as Java
> `short` might be even though it is signed. Thus i am inclined to remove that
> mapping.
>
> What if we say something to the effect of:
>
> > unless explicitly declared in the canonical layouts C's unsigned integral
> > types are mapped to the layouts associated with the required C's signed
> > integral types of the same bit sizes.
>
> ?
>
> Arguably C `unsigned short` could map to carriers Java `short` or Java
> `char`, but i am inclined to say the user should cast between Java `short` to
> `char` in such cases.
>
> FWIW i checked what the FFM API and jextract does today and it maps unsigned
> C types to signed Java types.
I tend to agree with your conclusion. And I confirm that we do not use "char"
anywhere in jextract. The only "problem" with that approach is that if we go
down that path, JAVA_CHAR is no longer a canonical type, so users cannot
mention it in function descriptors. Apart from requiring few test updates, I
don't see many other problems with it - if one really really wanted the result
of a native call to be converted to `char`, a MH adapter can be used.
Effectively, what you suggest amount at saying: we do have a JAVA_CHAR layout,
which is mostly there for Java interop. But a native linker (which only cares
about native interop) doesn't really care much about that. Does that sound good?
-
PR Comment: https://git.openjdk.org/jdk/pull/14037#issuecomment-1560086050
Re: RFR: 8308293: A linker should expose the layouts it supports [v4]
On Mon, 22 May 2023 14:34:32 GMT, Per Minborg wrote: > LGTM. Are there any additional types we might consider apart from the basic > ones and `size_t`? Maybe one for an address pointing at `errno`? I think there is a certain gravitational pull towards keeping the set of guaranteed canonical layouts as minimal as possible. In that sense pointer to errno seems not to meet the bar IMHO. (also note that one could always ask the captureStateLayout, and figure out how to express the errno type from there). - PR Comment: https://git.openjdk.org/jdk/pull/14037#issuecomment-1557585319
Re: RFR: 8308293: A linker should expose the layouts it supports [v4]
On Mon, 22 May 2023 09:34:53 GMT, Maurizio Cimadamore
wrote:
>> This patch adds an instance method on `Linker`, namely
>> `Linker::canonicalLayouts` which returns all the layouts known by the linker
>> as implementing some ABI type. For instance, if I call this on my machine
>> (Linux/x64) I get this:
>>
>>
>> jshell> import java.lang.foreign.*;
>>
>> jshell> Linker.nativeLinker().canonicalLayouts()
>> $2 ==> {char16_t=c16, int8_t=b8, long=j64, size_t=j64, bool=z8, int=i32,
>> long long=j64, int64_t=j64, void*=a64, float=f32, char=b8, int16_t=s16,
>> int32_t=i32, short=s16, double=d64}
>>
>>
>> This can be useful to discover the ABI types supported by a linker
>> implementation, as well as for, in the future, add support for more exotic
>> (and platform-dependent) linker types, such as `long double` or `complex
>> long`.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Address further review comments
LGTM. Are there any additional types we might consider apart from the basic
ones and `size_t`? Maybe one for an address pointing at `errno`?
-
Marked as reviewed by pminborg (Committer).
PR Review: https://git.openjdk.org/jdk/pull/14037#pullrequestreview-1436779774
Re: RFR: 8308293: A linker should expose the layouts it supports [v4]
> This patch adds an instance method on `Linker`, namely
> `Linker::canonicalLayouts` which returns all the layouts known by the linker
> as implementing some ABI type. For instance, if I call this on my machine
> (Linux/x64) I get this:
>
>
> jshell> import java.lang.foreign.*;
>
> jshell> Linker.nativeLinker().canonicalLayouts()
> $2 ==> {char16_t=c16, int8_t=b8, long=j64, size_t=j64, bool=z8, int=i32, long
> long=j64, int64_t=j64, void*=a64, float=f32, char=b8, int16_t=s16,
> int32_t=i32, short=s16, double=d64}
>
>
> This can be useful to discover the ABI types supported by a linker
> implementation, as well as for, in the future, add support for more exotic
> (and platform-dependent) linker types, such as `long double` or `complex
> long`.
Maurizio Cimadamore has updated the pull request incrementally with one
additional commit since the last revision:
Address further review comments
-
Changes:
- all: https://git.openjdk.org/jdk/pull/14037/files
- new: https://git.openjdk.org/jdk/pull/14037/files/df92467c..dbc8049a
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=14037=03
- incr: https://webrevs.openjdk.org/?repo=jdk=14037=02-03
Stats: 5 lines in 1 file changed: 3 ins; 0 del; 2 mod
Patch: https://git.openjdk.org/jdk/pull/14037.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14037/head:pull/14037
PR: https://git.openjdk.org/jdk/pull/14037
