RFR: 8254231: Implementation of Foreign Linker API (Incubator)

2020-10-13 Thread Maurizio Cimadamore
This patch contains the changes associated with the first incubation round of the foreign linker access API incubation (see JEP 389 [1]). This work is meant to sit on top of the foreign memory access support (see JEP 393 [2] and associated pull request [3]). The main goal of this API is to provi

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator)

2020-10-15 Thread Jorn Vernee
On Tue, 13 Oct 2020 13:08:14 GMT, Maurizio Cimadamore wrote: > This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2]

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v2]

2020-10-15 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and > associated pull request [3]). > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v3]

2020-10-15 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and > associated pull request [3]). > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-15 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and > associated pull request [3]). > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-15 Thread Coleen Phillimore
On Thu, 15 Oct 2020 17:08:28 GMT, Maurizio Cimadamore wrote: >> This patch contains the changes associated with the first incubation round >> of the foreign linker access API incubation >> (see JEP 389 [1]). This work is meant to sit on top of the foreign memory >> access support (see JEP 393

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-16 Thread Jorn Vernee
On Thu, 15 Oct 2020 22:39:50 GMT, Coleen Phillimore wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Re-add file erroneously deleted (detected as rename) > > src/hotspot/cpu/x86/foreign_globals_x86.cpp line 2

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-16 Thread Jorn Vernee
On Thu, 15 Oct 2020 22:44:54 GMT, Coleen Phillimore wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Re-add file erroneously deleted (detected as rename) > > src/hotspot/cpu/x86/foreign_globals_x86.hpp line 3

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-16 Thread Jorn Vernee
On Thu, 15 Oct 2020 22:52:14 GMT, Coleen Phillimore wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Re-add file erroneously deleted (detected as rename) > > src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp line

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-16 Thread Jorn Vernee
On Thu, 15 Oct 2020 22:42:49 GMT, Coleen Phillimore wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Re-add file erroneously deleted (detected as rename) > > src/hotspot/cpu/x86/foreign_globals_x86.cpp line 5

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-18 Thread David Holmes
Hi Jorn, I'm not reviewing this but this exchange caught my attention ... On 16/10/2020 9:15 pm, Jorn Vernee wrote: On Thu, 15 Oct 2020 22:42:49 GMT, Coleen Phillimore wrote: Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-19 Thread Jorn Vernee
On Fri, 16 Oct 2020 11:12:01 GMT, Jorn Vernee wrote: >> src/hotspot/cpu/x86/foreign_globals_x86.cpp line 56: >> >>> 54: } >>> 55: >>> 56: const ABIDescriptor parseABIDescriptor(JNIEnv* env, jobject jabi) { >> >> I don't know if you care about performance but of these env->calls >> transition i

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-19 Thread Jorn Vernee
On Thu, 15 Oct 2020 23:15:07 GMT, Coleen Phillimore wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Re-add file erroneously deleted (detected as rename) > > I looked through some Hotspot runtime code and tha

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v5]

2020-10-19 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and > associated pull request [3]). > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v6]

2020-10-19 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and > associated pull request [3]). > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-20 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and > associated pull request [3]). > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v8]

2020-10-21 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and associated pull request [3]). > > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v9]

2020-10-21 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and associated pull request [3]). > > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

2020-10-21 Thread Jorn Vernee
On Mon, 19 Oct 2020 11:24:45 GMT, Jorn Vernee wrote: >> I looked through some Hotspot runtime code and that looks ok. I saw a >> couple of strange things on my way through the code. See comments. > > Hi David, this code somewhat predates me, so I initially kept the JVM_ENTRY > since that was

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v9]

2020-10-21 Thread Paul Sandoz
On Wed, 21 Oct 2020 11:33:27 GMT, Maurizio Cimadamore wrote: >> This patch contains the changes associated with the first incubation round >> of the foreign linker access API incubation >> (see JEP 389 [1]). This work is meant to sit on top of the foreign memory >> access support (see JEP 393

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-21 Thread Paul Sandoz
On Tue, 20 Oct 2020 17:23:26 GMT, Maurizio Cimadamore wrote: >> This patch contains the changes associated with the first incubation round >> of the foreign linker access API incubation >> (see JEP 389 [1]). This work is meant to sit on top of the foreign memory >> access support (see JEP 393

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-22 Thread Maurizio Cimadamore
On Tue, 20 Oct 2020 21:31:17 GMT, Paul Sandoz wrote: >> Maurizio Cimadamore has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 25 commits: >> >> - Merge branch 'master' into 8254231_linker >> - Fix incorrect capitalization in one c

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v9]

2020-10-22 Thread Maurizio Cimadamore
On Wed, 21 Oct 2020 16:23:16 GMT, Paul Sandoz wrote: >> Maurizio Cimadamore has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 27 commits: >> >> - Merge branch 'master' into 8254231_linker >> - Don't use JNI when generating native

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-22 Thread Jorn Vernee
On Tue, 20 Oct 2020 21:08:26 GMT, Paul Sandoz wrote: >> Maurizio Cimadamore has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 25 commits: >> >> - Merge branch 'master' into 8254231_linker >> - Fix incorrect capitalization in one c

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-22 Thread Jorn Vernee
On Tue, 20 Oct 2020 21:53:55 GMT, Paul Sandoz wrote: >> Maurizio Cimadamore has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 25 commits: >> >> - Merge branch 'master' into 8254231_linker >> - Fix incorrect capitalization in one c

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-22 Thread Paul Sandoz
On Thu, 22 Oct 2020 13:30:13 GMT, Maurizio Cimadamore wrote: >> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java >> line 126: >> >>> 124: * >>> 125: * @param symbol downcall symbol. >>> 126: * @param type the method type. >> >> s/method type/carr

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-22 Thread Paul Sandoz
On Thu, 22 Oct 2020 14:31:12 GMT, Jorn Vernee wrote: >> src/java.base/share/classes/java/lang/invoke/NativeMethodHandle.java line 36: >> >>> 34: import static java.lang.invoke.MethodHandleStatics.newInternalError; >>> 35: >>> 36: /** TODO */ >> >> Is the TODO to make this class public later an

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v9]

2020-10-22 Thread Paul Sandoz
On Thu, 22 Oct 2020 14:26:37 GMT, Maurizio Cimadamore wrote: >> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractNativeScope.java >> line 120: >> >>> 118: } >>> 119: } >>> 120: throw new AssertionError("Cannot get here!"); >> >> This

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v10]

2020-10-22 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and associated pull request [3]). > > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-22 Thread Maurizio Cimadamore
On Wed, 21 Oct 2020 19:05:42 GMT, Paul Sandoz wrote: >> Maurizio Cimadamore has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 25 commits: >> >> - Merge branch 'master' into 8254231_linker >> - Fix incorrect capitalization in one c

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-22 Thread Maurizio Cimadamore
On Thu, 22 Oct 2020 15:46:33 GMT, Paul Sandoz wrote: >> IIRC this was added to silence a javac linter warning. Something should be >> added here. There is/was no plan to make this class public though. > > It's odd the lint warning is triggering on a package private class and > private methods.

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v9]

2020-10-22 Thread Maurizio Cimadamore
On Wed, 21 Oct 2020 17:42:53 GMT, Paul Sandoz wrote: > Some design considerations, to consider later maybe. > > The IR representation could be simplified to use record classes (which should > be exiting preview in 16), implementing a Binding interface. The interpreter > and specializer (compil

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v7]

2020-10-22 Thread Jorn Vernee
On Thu, 22 Oct 2020 16:31:00 GMT, Maurizio Cimadamore wrote: >> Some of this is familiar to me from reviews in the `panama-foreign` >> repository, but less so than the memory API. I focused on the Java code, and >> ignored changes that are common with the memory API PR. >> >> If it helps in c

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v11]

2020-10-22 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and associated pull request [3]). > > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v12]

2020-10-22 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and associated pull request [3]). > > The main goal of this API

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v12]

2020-10-23 Thread Magnus Ihse Bursie
On Thu, 22 Oct 2020 17:04:34 GMT, Maurizio Cimadamore wrote: >> This patch contains the changes associated with the first incubation round >> of the foreign linker access API incubation >> (see JEP 389 [1]). This work is meant to sit on top of the foreign memory >> access support (see JEP 393

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v12]

2020-10-23 Thread Maurizio Cimadamore
On Fri, 23 Oct 2020 11:02:11 GMT, Magnus Ihse Bursie wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix whitespaces > > Changes requested by ihse (Reviewer). @magicus the files you commented on are not par