Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Fri, 13 May 2022 19:13:46 GMT, Vladimir Ivanov wrote: >> Ok. Then, if no one objects, I will leave this area as-is for now. (and >> perhaps come back to this issue in the future, if it becomes more pressing). >> >> (I'll also note that this issue already exists in the current code that's in >> mainline. So, it seems fair to address this as a followup as well, if >> needed). > > I don't see a way to reliably handle async exceptions purely with > `try/catch`. In the end, there's always a safepoint poll right before > returning from a method where new exception can be installed. So, there's > always a small chance present to observe a pending exception on VM side > irrespective of how hard you try on Java side. > > From reliability perspective, I find it important to gracefully handle such > corner cases. But I'm fine with addressing the problem separately. > > As an alternative solution, exception handling for upcalls can be handled by > another upcall (into catch handler when pending exception is encountered). As > a bonus, it allows to handle repeated exceptions. In the worst case, it would > manifest as a hang (when async exceptions are continuously delivered). Still, > some special handling is needed for stack overflow errors. (Not sure how > those are handled now. Are they?) SOE (of the Java exception kind) is not specially handled right now. I think the same rule applies there: we can't unwind or return to native frames (at least not without some guidance from the user). I've filed an issue here to capture some of the discussion: https://bugs.openjdk.java.net/browse/JDK-8286761 - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Thu, 12 May 2022 17:26:44 GMT, Jorn Vernee wrote: >>> Do nothing special for async exceptions. i.e. if they happen anywhere >>> between 1. and 6. they will end up in one of the fallback handlers and the >>> VM will be terminated. >> >> My understanding is that if they materialize while we're executing the >> upcall Java code, if that code has a try/catch block, we will go there, >> rather than crash the VM. >> >> In other words, IMHO the only problem with async exception is if they occur >> _after_ the Java user code has completed, because that will crash the Java >> adapter, this preventing it from returning to native call cleanly. >> >> So, either we disable async exceptions during that phase (e.g. after user >> code has executed, but before we return back to native code), or we just >> punt and stop. Since this seems like a corner^3 case, and since there are >> also other issue with upcalls that can occur if other threads do not >> cooperate (e.g. an upcall can get stuck into an infinite safepoint if the VM >> exits while an async native thread runs the upcall), and given that >> obtaining a linker is a restricted operation anyway, I don't think we should >> bend over backwards to try to add 1% more safety to something that's >> unavoidably sharp anyways. > > Ok. Then, if no one objects, I will leave this area as-is for now. (and > perhaps come back to this issue in the future, if it becomes more pressing). > > (I'll also note that this issue already exists in the current code that's in > mainline. So, it seems fair to address this as a followup as well, if needed). I don't see a way to reliably handle async exceptions purely with `try/catch`. In the end, there's always a safepoint poll right before returning from a method where new exception can be installed. So, there's always a small chance present to observe a pending exception on VM side irrespective of how hard you try on Java side. >From reliability perspective, I find it important to gracefully handle such >corner cases. But I'm fine with addressing the problem separately. As an alternative solution, exception handling for upcalls can be handled by another upcall (into catch handler when pending exception is encountered). As a bonus, it allows to handle repeated exceptions. In the worst case, it would manifest as a hang (when async exceptions are continuously delivered). Still, some special handling is needed for stack overflow errors. (Not sure how those are handled now. Are they?) - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Thu, 12 May 2022 12:10:53 GMT, Maurizio Cimadamore wrote: >> Okay, I see. I think I acted a little too hastily on this yesterday. I'll >> revert the change that uses this blocking mechanism. >> >> The stack more or less looks like this during an upcall: >> >> >> | --- >> | | >> | --- >> | <1: user define try block with exception handler (maybe)> | >> | --- >> | <2: user code start> | >> | --- >> | <3: method handle impl frames 1> | >> | --- >> | <4: upcall wrapper class with fallback handler 1> | >> | --- >> | <5: method handle impl frames 2> | >> | --- >> | <6: upcallk stub with fallback handler 2> | >> | <7: unknown native code> >> | --- >> | | >> >> >> I think there are several options to address async exceptions: >> 1. Do nothing special for async exceptions. i.e. if they happen anywhere >> between 1. and 6. they will end up in one of the fallback handlers and the >> VM will be terminated. >> 2. Block async exceptions in all code up from 6. >> 3. Somehow only block async exceptions only between 6. and 1. >> I think that is possible by changing the API so that the user passes us >> a method handle to their fallback exception handler, and we invoke it in our >> code in 4. We would need 2 methods for blocking and unblocking async >> exceptions from Java. Then we could disable async exceptions at the start of >> 6. enabled them at the start of the try block in 4. (around the call to user >> code), and disable them at the end of this try block. Then finally re-enable >> them at the end of 6. If an exception occurs in the try block in 4., >> delegate to the user-defined exception handler (but use the VM terminate >> strategy as a fallback for when another exception occurs). The other problem >> I see with that is that to make that fast enough (i.e. not incur a ~1.5-2x >> cost on call overhead), we would need compiler intrinsics for the >> blocking/unblocking, and in the past we've been unable to define 'critical' >> sections of code like that in C2 (it's an unsolved problem at this point). > >> Do nothing special for async exceptions. i.e. if they happen anywhere >> between 1. and 6. they will end up in one of the fallback handlers and the >> VM will be terminated. > > My understanding is that if they materialize while we're executing the upcall > Java code, if that code has a try/catch block, we will go there, rather than > crash the VM. > > In other words, IMHO the only problem with async exception is if they occur > _after_ the Java user code has completed, because that will crash the Java > adapter, this preventing it from returning to native call cleanly. > > So, either we disable async exceptions during that phase (e.g. after user > code has executed, but before we return back to native code), or we just punt > and stop. Since this seems like a corner^3 case, and since there are also > other issue with upcalls that can occur if other threads do not cooperate > (e.g. an upcall can get stuck into an infinite safepoint if the VM exits > while an async native thread runs the upcall), and given that obtaining a > linker is a restricted operation anyway, I don't think we should bend over > backwards to try to add 1% more safety to something that's unavoidably sharp > anyways. Ok. Then, if no one objects, I will leave this area as-is for now. (and perhaps come back to this issue in the future, if it becomes more pressing). - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Thu, 12 May 2022 09:24:23 GMT, Jorn Vernee wrote: > Do nothing special for async exceptions. i.e. if they happen anywhere between > 1. and 6. they will end up in one of the fallback handlers and the VM will be > terminated. My understanding is that if they materialize while we're executing the upcall Java code, if that code has a try/catch block, we will go there, rather than crash the VM. In other words, IMHO the only problem with async exception is if they occur _after_ the Java user code has completed, because that will crash the Java adapter, this preventing it from returning to native call cleanly. So, either we disable async exceptions during that phase (e.g. after user code has executed, but before we return back to native code), or we just punt and stop. Since this seems like a corner^3 case, and since there are also other issue with upcalls that can occur if other threads do not cooperate (e.g. an upcall can get stuck into an infinite safepoint if the VM exits while an async native thread runs the upcall), and given that obtaining a linker is a restricted operation anyway, I don't think we should bend over backwards to try to add 1% more safety to something that's unavoidably sharp anyways. - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Thu, 12 May 2022 03:32:15 GMT, David Holmes wrote: >> Is it possible for these upcalls to be nested? If yes, we could add a >> boolean to context to avoid unsetting the flag in those nested cases. And >> now that I think we should probably add that check in >> NoAsyncExceptionDeliveryMark too if we allow broader use of this flag. David >> added the NoAsyncExceptionDeliveryMark code with that assert about nesting >> so maybe he might have more insights about that. > > NoAsyncExceptionDeliveryMark is not for general use! There is no provision > for blocking async exceptions when running user-defined Java code. > NoAsyncExceptionDeliveryMark was purely for protecting "system Java code". Okay, I see. I think I acted a little too hastily on this yesterday. I'll revert the change that uses this blocking mechanism. The stack more or less looks like this during an upcall: | --- | | | --- | <1: user define try block with exception handler (maybe)> | | --- | <2: user code start> | | --- | <3: method handle impl frames 1> | | --- | <4: upcall wrapper class with fallback handler 1> | | --- | <5: method handle impl frames 2> | | --- | <6: upcallk stub with fallback handler 2> | | <7: unknown native code> | --- | | I think there are several options to address async exceptions: 1. Do nothing special for async exceptions. i.e. if they happen anywhere between 1. and 6. they will up in one of the fallback handlers and the VM will be terminated. 2. Block async exceptions in all code up from 6. 3. Somehow only block async exceptions only between 6. and 1. I think that is possible by changing the API so that the user passes us a method handle to their fallback exception handler. We would need 2 methods for blocking and unblocking async exceptions from Java. Then we could disable async exceptions at the start of 6. enabled them at the start of the try block in 4. (around the call to user code), and disable them at the end of this try block. Then finally re-enable them at the end of 6. If an exception occurs in the try block in 4., delegate to the user-defined exception handler (but use the VM terminate strategy as a fallback for when another exception occurs). The other problem I see with that is that to make that fast enough (i.e. not incur a ~1.5-2x cost on call overhead), we would need compiler intrinsics for the blocking/unblocking, and in the past we've been unable to define 'critical' sections of code like that in C2 (it's an unsolved problem at this point). - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Wed, 11 May 2022 20:27:42 GMT, Patricio Chilano Mateo wrote: >> I went ahead and implemented this suggestion. Now we block async exceptions >> in on_entry, and unblock in on_exit. > > Is it possible for these upcalls to be nested? If yes, we could add a boolean > to context to avoid unsetting the flag in those nested cases. And now that I > think we should probably add that check in NoAsyncExceptionDeliveryMark too > if we allow broader use of this flag. David added the > NoAsyncExceptionDeliveryMark code with that assert about nesting so maybe he > might have more insights about that. NoAsyncExceptionDeliveryMark is not for general use! There is no provision for blocking async exceptions when running user-defined Java code. NoAsyncExceptionDeliveryMark was purely for protecting "system Java code". - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Wed, 11 May 2022 17:55:16 GMT, Jorn Vernee wrote: >> Oh nice! I was just thinking that the only possible way out of this >> conundrum would be to somehow block the delivery of async exceptions (at >> least outside of the user's exception handler). So, that seems to be exactly >> what we need :) > > I went ahead and implemented this suggestion. Now we block async exceptions > in on_entry, and unblock in on_exit. Is it possible for these upcalls to be nested? If yes, we could add a boolean to context to avoid unsetting the flag in those nested cases. And now that I think we should probably add that check in NoAsyncExceptionDeliveryMark too if we allow broader use of this flag. David added the NoAsyncExceptionDeliveryMark code with that assert about nesting so maybe he might have more insights about that. - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Wed, 11 May 2022 16:38:54 GMT, Jorn Vernee wrote: >> If you want to avoid processing asynchronous exceptions during this upcall >> you could block them (check NoAsyncExceptionDeliveryMark in >> JavaThread::exit()). Seems you could set the flag in >> ProgrammableUpcallhandler::on_entry() and unset it back on >> ProgrammableUpcallhandler::on_exit(). While that flag is set any >> asynchronous exception in the handshake queue of this thread will be skipped >> from processing. Maybe we should add a public method in the JavaThread >> class, block_async_exceptions()/unblock_async_exceptions() so we hide the >> handshake implementation. > > Oh nice! I was just thinking that the only possible way out of this conundrum > would be to somehow block the delivery of async exceptions (at least outside > of the user's exception handler). So, that seems to be exactly what we need :) I went ahead and implemented this suggestion. Now we block async exceptions in on_entry, and unblock in on_exit. - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Wed, 11 May 2022 16:20:32 GMT, Patricio Chilano Mateo wrote: >> Discussed this with Maurizio as well. >> >> My understanding is as follows: >> 1. Async exceptions are installed into a thread's `pending_exception` field >> by handshake at a safepoint >> 2. From there they are "thrown" (I guess during the same safepoint?), in >> which case we either end up in a user-defined exception handler (higher up >> the stack), in our Java fallback exception handler (print stack trace and >> terminate), or in this fallback exception handler (print and terminate). >> 3. If we end up in this exception handler it means the async exception was >> installed somewhere outside of our Java exception handler and "thrown" from >> there. However, it also means that the Java code we were calling into >> completed abruptly. >> 4. We've previously established that we have no way of signalling to the >> native code that is calling us that something went wrong, and so the only >> safe option is to terminate, as to not leave the application in an >> inconsistent state. >> >> As a consequence, I don't think we have much choice in the case of async >> exceptions if we get here. Silently clearing them seems like it will leave >> the program in an inconsistent state (since we unwound some frames), so we >> have to terminate I think. >> >> (@dholmes-ora is my understanding of async exceptions in point 1. and 2. >> correct here?) > > If you want to avoid processing asynchronous exceptions during this upcall > you could block them (check NoAsyncExceptionDeliveryMark in > JavaThread::exit()). Seems you could set the flag in > ProgrammableUpcallhandler::on_entry() and unset it back on > ProgrammableUpcallhandler::on_exit(). While that flag is set any asynchronous > exception in the handshake queue of this thread will be skipped from > processing. Maybe we should add a public method in the JavaThread class, > block_async_exceptions()/unblock_async_exceptions() so we hide the handshake > implementation. Oh nice! I was just thinking that the only possible way out of this conundrum would be to somehow block the delivery of async exceptions (at least outside of the user's exception handler). So, that seems to be exactly what we need :) - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Wed, 11 May 2022 15:44:19 GMT, Jorn Vernee wrote: >> We have an exception handler in Java as well, so this code is only a fail >> safe. But, I think in the case of asynchronous exceptions this might be >> problematic if the exception is discovered by the current thread outside of >> the Java exception handler, turned into a synchronous exception and then we >> get here and call `ProgrammableUpcallhandler::handle_uncaught_exception` and >> then crash. Or if the asynchronous exception is discovered in >> `ProgrammableUpcallHandler::on_exit` (where there is currently an assert for >> no exceptions). >> >> I think you're right that, in both of those cases, if the exception is >> asynchronous, we should just ignore it. > > Discussed this with Maurizio as well. > > My understanding is as follows: > 1. Async exceptions are installed into a thread's `pending_exception` field > by handshake at a safepoint > 2. From there they are "thrown" (I guess during the same safepoint?), in > which case we either end up in a user-defined exception handler (higher up > the stack), in our Java fallback exception handler (print stack trace and > terminate), or in this fallback exception handler (print and terminate). > 3. If we end up in this exception handler it means the async exception was > installed somewhere outside of our Java exception handler and "thrown" from > there. However, it also means that the Java code we were calling into > completed abruptly. > 4. We've previously established that we have no way of signalling to the > native code that is calling us that something went wrong, and so the only > safe option is to terminate, as to not leave the application in an > inconsistent state. > > As a consequence, I don't think we have much choice in the case of async > exceptions if we get here. Silently clearing them seems like it will leave > the program in an inconsistent state (since we unwound some frames), so we > have to terminate I think. > > (@dholmes-ora is my understanding of async exceptions in point 1. and 2. > correct here?) If you want to avoid processing asynchronous exceptions during this upcall you could block them (check NoAsyncExceptionDeliveryMark in JavaThread::exit()). Seems you could set the flag in ProgrammableUpcallhandler::on_entry() and unset it back on ProgrammableUpcallhandler::on_exit(). While that flag is set any asynchronous exception in the handshake queue of this thread will be skipped from processing. Maybe we should add a public method in the JavaThread class, block_async_exceptions()/unblock_async_exceptions() so we hide the handshake implementation. - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Wed, 11 May 2022 12:05:29 GMT, Jorn Vernee wrote: >> src/hotspot/cpu/aarch64/universalUpcallHandler_aarch64.cpp line 306: >> >>> 304: intptr_t exception_handler_offset = __ pc() - start; >>> 305: >>> 306: // Native caller has no idea how to handle exceptions, >> >> Can you elaborate, please, how it is expected to work in presence of >> asynchronous exceptions? I'd expect to see a code which unconditionally >> clears pending exception with an assertion that verifies that the exception >> is of expected type. > > We have an exception handler in Java as well, so this code is only a fail > safe. But, I think in the case of asynchronous exceptions this might be > problematic if the exception is discovered by the current thread outside of > the Java exception handler, turned into a synchronous exception and then we > get here and call `ProgrammableUpcallhandler::handle_uncaught_exception` and > then crash. Or if the asynchronous exception is discovered in > `ProgrammableUpcallHandler::on_exit` (where there is currently an assert for > no exceptions). > > I think you're right that, in both of those cases, if the exception is > asynchronous, we should just ignore it. Discussed this with Maurizio as well. My understanding is as follows: 1. Async exceptions are installed into a thread's `pending_exception` field by handshake at a safepoint 2. From there they are "thrown" (I guess during the same safepoint?), in which case we either end up in a user-defined exception handler (higher up the stack), in our Java fallback exception handler (print stack trace and terminate), or in this fallback exception handler (print and terminate). 3. If we end up in this exception handler it means the async exception was installed somewhere outside of our Java exception handler and "thrown" from there. However, it also means that the Java code we were calling into completed abruptly. 4. We've previously established that we have no way of signalling to the native code that is calling us that something went wrong, and so the only safe option is to terminate, as to not leave the application in an inconsistent state. As a consequence, I don't think we have much choice in the case of async exceptions if we get here. Silently clearing them seems like it will leave the program in an inconsistent state (since we unwound some frames), so we have to terminate I think. (@dholmes-ora is my understanding of async exceptions in point 1. and 2. correct here?) - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 20:38:05 GMT, Vladimir Ivanov 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 21 commits: >> >> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2 >> - Remove unneeded ComputeMoveOrder >> - Remove comment about native calls in lcm.cpp >> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64 >> >>Reviewed-by: jvernee, mcimadamore >> - Update riscv and arm stubs >> - Remove spurious ProblemList change >> - Pass pointer to LogStream >> - Polish >> - Replace TraceNativeInvokers flag with unified logging >> - Fix other platforms, take 2 >> - ... and 11 more: >> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91 > > src/hotspot/share/prims/foreign_globals.cpp line 217: > >> 215: >> 216: public: >> 217: ForeignCMO(int total_in_args, const VMRegPair* in_regs, int >> total_out_args, VMRegPair* out_regs, > > I propose to turn it into a trivial ctor and move all the logic into a helper > static function which returns the computed moves. Done. Moved constructor logic into a `compute` method, and added a static helper that constructs a ComputeMoveOrder, calls `compute`, and then returns the moves. - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Wed, 11 May 2022 11:06:51 GMT, Jorn Vernee wrote: >> src/hotspot/share/opto/callGenerator.cpp line 1131: >> >>> 1129: >>> 1130: case vmIntrinsics::_linkToNative: >>> 1131: print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(), >> >> Why is it unconditionally reported as inlining failure? > > The call that is being processed here is `linkToNative`, and that call is not > inlined, so reporting an inlining failure seems correct. We still go through > the method handle trampoline stub which loads the actual target from the > NativeEntryPoint appendix argument (`jump_to_native_invoker` in > methodHandles_x86.cpp). > > It's potentially faster here to generate a runtime call to the underlying > invoker/downcall stub if the NativeEntryPoint is constant (i.e. avoid the > lookup through NEP in the MH trampoline), but I hadn't gotten to > investigating that yet. > > From comparing the benchmark times between this and the old implementation > (which generated an inline call), they are not all that different. So it > seemed that doing something special here would not save that much time any > ways. (but, still would be good to investigate at some point) I've filed: https://bugs.openjdk.java.net/browse/JDK-8286588 - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 21:02:39 GMT, Vladimir Ivanov 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 21 commits: >> >> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2 >> - Remove unneeded ComputeMoveOrder >> - Remove comment about native calls in lcm.cpp >> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64 >> >>Reviewed-by: jvernee, mcimadamore >> - Update riscv and arm stubs >> - Remove spurious ProblemList change >> - Pass pointer to LogStream >> - Polish >> - Replace TraceNativeInvokers flag with unified logging >> - Fix other platforms, take 2 >> - ... and 11 more: >> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91 > > Nice work! Looks good. Some minor comments/questions follow. @iwanowww Thanks for the review! I've addressed most comments already, but will have to think a bit on how to handle the asynchronous exceptions issue. > src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5531: > >> 5529: } >> 5530: >> 5531: // On 64 bit we will store integer like items to the stack as > > Time for a cleanup? `64 bit` vs `64bit`, `abi`, `Aarch64`. I've cleaned up the spaces and capitalization here a bit. > src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp line 36: > >> 34: #include "code/nativeInst.hpp" >> 35: #include "code/vtableStubs.hpp" >> 36: #include "compiler/disassembler.hpp" > > Redundant includes? No new code added in the file. Good catch. Seems like a merge artifact maybe. Removing them seems to be fine. - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 20:53:37 GMT, Vladimir Ivanov wrote: >> src/hotspot/share/utilities/growableArray.hpp line 151: >> >>> 149: return _data; >>> 150: } >>> 151: >> >> This accessor is added to be able to temporarily view a stable GrowableArray >> instance as a C-style array. It is used to by `NativeCallConv` and >> `RegSpiller` in `foreign_globals.hpp`. >> >> GrowableArray already has an `adr_at` accessor that does something similar, >> but using `adr_at(0)` fails on empty growable arrays since it also performs >> a bounds check. So it can not be used. > > Any problems with migrating `CallConv` and `RegSpiller`away from ` VMReg* + > int` to `GrowableArray`? I'll try migrating to `GrowableArray` - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 20:45:02 GMT, Vladimir Ivanov 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 21 commits: >> >> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2 >> - Remove unneeded ComputeMoveOrder >> - Remove comment about native calls in lcm.cpp >> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64 >> >>Reviewed-by: jvernee, mcimadamore >> - Update riscv and arm stubs >> - Remove spurious ProblemList change >> - Pass pointer to LogStream >> - Polish >> - Replace TraceNativeInvokers flag with unified logging >> - Fix other platforms, take 2 >> - ... and 11 more: >> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91 > > src/hotspot/cpu/x86/foreign_globals_x86_64.cpp line 52: > >> 50: >> 51: objArrayOop inputStorage = >> jdk_internal_foreign_abi_ABIDescriptor::inputStorage(abi_oop); >> 52: loadArray(inputStorage, INTEGER_TYPE, abi._integer_argument_registers, >> as_Register); > > `loadArray` helper looks a bit misleading. In presence of `javaClass`-style > accessors, it misleadingly hints that it refers to some Java-level > operation/entity, though what it does it parses register list representation > backed by a Java array. I suggest to rename it to something like > `parse_argument_registers_array()`). I went with `parse_register_array` (since it's also used for return registers) - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Wed, 11 May 2022 10:51:10 GMT, Jorn Vernee wrote: >> src/hotspot/cpu/x86/foreign_globals_x86.hpp line 30: >> >>> 28: #include "utilities/growableArray.hpp" >>> 29: >>> 30: class outputStream; >> >> Redundant declaration? > > Yeah, this whole file is redundant :) (replaced by foreign_globals_x86_64.hpp) Hmm, it doesn't look like having x64 specific header files is support by the CPU_HEADER macros. Will stick to the shared header file for x86_32 and x86_64 for now. - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 21:01:48 GMT, Vladimir Ivanov 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 21 commits: >> >> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2 >> - Remove unneeded ComputeMoveOrder >> - Remove comment about native calls in lcm.cpp >> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64 >> >>Reviewed-by: jvernee, mcimadamore >> - Update riscv and arm stubs >> - Remove spurious ProblemList change >> - Pass pointer to LogStream >> - Polish >> - Replace TraceNativeInvokers flag with unified logging >> - Fix other platforms, take 2 >> - ... and 11 more: >> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91 > > src/hotspot/cpu/aarch64/universalUpcallHandler_aarch64.cpp line 306: > >> 304: intptr_t exception_handler_offset = __ pc() - start; >> 305: >> 306: // Native caller has no idea how to handle exceptions, > > Can you elaborate, please, how it is expected to work in presence of > asynchronous exceptions? I'd expect to see a code which unconditionally > clears pending exception with an assertion that verifies that the exception > is of expected type. We have an exception handler in Java as well, so this code is only a fail safe. But, I think in the case of asynchronous exceptions this might be problematic if the exception is discovered by the current thread outside of the Java exception handler, turned into a synchronous exception and then we get here and call `ProgrammableUpcallhandler::handle_uncaught_exception` and then crash. Or if the asynchronous exception is discovered in `ProgrammableUpcallHandler::on_exit` (where there is currently an assert for no exceptions). I think you're right that, in both of those cases, if the exception is asynchronous, we should just ignore it. - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 20:48:47 GMT, Vladimir Ivanov 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 21 commits:
>>
>> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2
>> - Remove unneeded ComputeMoveOrder
>> - Remove comment about native calls in lcm.cpp
>> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64
>>
>>Reviewed-by: jvernee, mcimadamore
>> - Update riscv and arm stubs
>> - Remove spurious ProblemList change
>> - Pass pointer to LogStream
>> - Polish
>> - Replace TraceNativeInvokers flag with unified logging
>> - Fix other platforms, take 2
>> - ... and 11 more:
>> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91
>
> src/hotspot/share/prims/foreign_globals.hpp line 35:
>
>> 33: #include CPU_HEADER(foreign_globals)
>> 34:
>> 35: class CallConvClosure {
>
> Just a question on terminology: why is it called a `Closure`?
It is the terminology used in other parts of hotspot for function objects it
seems. See for instance the classes in `iterator.hpp`
> src/hotspot/share/prims/foreign_globals.hpp line 62:
>
>> 60:
>> 61:
>> 62: class JavaCallConv : public CallConvClosure {
>
> Does it really worth to abbreviate `CallingConvention` to `CallConv`?
Maybe not... I'll spell out the full thing.
-
PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 20:30:09 GMT, Vladimir Ivanov 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 21 commits:
>>
>> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2
>> - Remove unneeded ComputeMoveOrder
>> - Remove comment about native calls in lcm.cpp
>> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64
>>
>>Reviewed-by: jvernee, mcimadamore
>> - Update riscv and arm stubs
>> - Remove spurious ProblemList change
>> - Pass pointer to LogStream
>> - Polish
>> - Replace TraceNativeInvokers flag with unified logging
>> - Fix other platforms, take 2
>> - ... and 11 more:
>> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91
>
> src/hotspot/share/prims/foreign_globals.cpp line 147:
>
>> 145: // based on ComputeMoveOrder from x86_64 shared runtime code.
>> 146: // with some changes.
>> 147: class ForeignCMO: public StackObj {
>
> Considering how seldom it is used, I don't see much value in abbreviating it.
> Also, the comment is misleading: there's no such entity as `ComputeMoveOrder`
> in the code. And `compute_move_order` is completely removed by this change.
Good points, I think we can just rename this class to `ComputeMoveOrder` at
this point.
-
PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 19:21:58 GMT, Vladimir Ivanov 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 21 commits: >> >> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2 >> - Remove unneeded ComputeMoveOrder >> - Remove comment about native calls in lcm.cpp >> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64 >> >>Reviewed-by: jvernee, mcimadamore >> - Update riscv and arm stubs >> - Remove spurious ProblemList change >> - Pass pointer to LogStream >> - Polish >> - Replace TraceNativeInvokers flag with unified logging >> - Fix other platforms, take 2 >> - ... and 11 more: >> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91 > > src/hotspot/share/opto/callGenerator.cpp line 1131: > >> 1129: >> 1130: case vmIntrinsics::_linkToNative: >> 1131: print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(), > > Why is it unconditionally reported as inlining failure? The call that is being processed here is `linkToNative`, and that call is not inlined, so reporting an inlining failure seems correct. We still go through the method handle trampoline stub which loads the actual target from the NativeEntryPoint (`jump_to_native_invoker` in methodHandles_x86.cpp). It's potentially faster here to generate a runtime call to the underlying invoker/downcall stub if the NativeEntryPoint is constant (i.e. avoid the lookup through NEP in the MH trampoline), but I hadn't gotten to investigating that yet. >From comparing the benchmark times between this and the old implementation >(which generated an inline call), they are not all that different. So it >seemed that doing something special here would not save that much time any >ways. (but, still would be good to investigate at some point) - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 19:16:35 GMT, Vladimir Ivanov 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 21 commits:
>>
>> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2
>> - Remove unneeded ComputeMoveOrder
>> - Remove comment about native calls in lcm.cpp
>> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64
>>
>>Reviewed-by: jvernee, mcimadamore
>> - Update riscv and arm stubs
>> - Remove spurious ProblemList change
>> - Pass pointer to LogStream
>> - Polish
>> - Replace TraceNativeInvokers flag with unified logging
>> - Fix other platforms, take 2
>> - ... and 11 more:
>> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91
>
> src/hotspot/share/code/codeBlob.hpp line 754:
>
>> 752: class ProgrammableUpcallHandler;
>> 753:
>> 754: class OptimizedEntryBlob: public RuntimeBlob {
>
> What's the motivation to move `OptimizedEntryBlob` up in the hierarchy (from
> `BufferBlob` to `RuntimeBlob`)?
Some of it is discussed here: https://github.com/openjdk/panama-foreign/pull/617
Essentially, it is to avoid accidentally inheriting behavior from BufferBlob
which we don't want. Also, BufferBlob currently expects a fixed-sized header
(`sizeof(BufferBlob)`), while OptimizedEntryBlobs has fields, so we'd have to
pass the real header size to the `BufferBlob` constructor, which is a bit messy.
It felt better to just cleanly break away from BufferBlob.
-
PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 18:55:03 GMT, Vladimir Ivanov 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 21 commits: >> >> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2 >> - Remove unneeded ComputeMoveOrder >> - Remove comment about native calls in lcm.cpp >> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64 >> >>Reviewed-by: jvernee, mcimadamore >> - Update riscv and arm stubs >> - Remove spurious ProblemList change >> - Pass pointer to LogStream >> - Polish >> - Replace TraceNativeInvokers flag with unified logging >> - Fix other platforms, take 2 >> - ... and 11 more: >> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91 > > src/hotspot/cpu/x86/foreign_globals_x86.hpp line 30: > >> 28: #include "utilities/growableArray.hpp" >> 29: >> 30: class outputStream; > > Redundant declaration? Yeah, this whole file is redundant :) (replaced by foreign_globals_x86_64.hpp) - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 18:48:08 GMT, Vladimir Ivanov 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 21 commits:
>>
>> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2
>> - Remove unneeded ComputeMoveOrder
>> - Remove comment about native calls in lcm.cpp
>> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64
>>
>>Reviewed-by: jvernee, mcimadamore
>> - Update riscv and arm stubs
>> - Remove spurious ProblemList change
>> - Pass pointer to LogStream
>> - Polish
>> - Replace TraceNativeInvokers flag with unified logging
>> - Fix other platforms, take 2
>> - ... and 11 more:
>> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5547:
>
>> 5545: } else if (dst.first()->is_stack()) {
>> 5546: // reg to stack
>> 5547: // Do we really have to sign extend???
>
> Obsolete? Remove?
Yes, this looks like it can be removed. (was copied over from
SharedRuntime_aarch64)
-
PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Tue, 10 May 2022 18:44:01 GMT, Vladimir Ivanov 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 21 commits: >> >> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2 >> - Remove unneeded ComputeMoveOrder >> - Remove comment about native calls in lcm.cpp >> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64 >> >>Reviewed-by: jvernee, mcimadamore >> - Update riscv and arm stubs >> - Remove spurious ProblemList change >> - Pass pointer to LogStream >> - Polish >> - Replace TraceNativeInvokers flag with unified logging >> - Fix other platforms, take 2 >> - ... and 11 more: >> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91 > > src/hotspot/cpu/aarch64/frame_aarch64.cpp line 379: > >> 377: // need unextended_sp here, since normal sp is wrong for interpreter >> callees >> 378: return reinterpret_cast( >> 379: reinterpret_cast(frame.unextended_sp()) + >> in_bytes(_frame_data_offset)); > > Maybe use `address` instead of `char*`? Ok. I think I used `char*` to try and avoid a potential strict-aliasing violation, but I don't think we compile with that turned on any ways. Will change it to `address` (for x86 too) - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Mon, 28 Mar 2022 12:15:10 GMT, Jorn Vernee 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 21 commits: >> >> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2 >> - Remove unneeded ComputeMoveOrder >> - Remove comment about native calls in lcm.cpp >> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64 >> >>Reviewed-by: jvernee, mcimadamore >> - Update riscv and arm stubs >> - Remove spurious ProblemList change >> - Pass pointer to LogStream >> - Polish >> - Replace TraceNativeInvokers flag with unified logging >> - Fix other platforms, take 2 >> - ... and 11 more: >> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91 > > src/hotspot/share/utilities/growableArray.hpp line 151: > >> 149: return _data; >> 150: } >> 151: > > This accessor is added to be able to temporarily view a stable GrowableArray > instance as a C-style array. It is used to by `NativeCallConv` and > `RegSpiller` in `foreign_globals.hpp`. > > GrowableArray already has an `adr_at` accessor that does something similar, > but using `adr_at(0)` fails on empty growable arrays since it also performs a > bounds check. So it can not be used. Any problems with migrating `CallConv` and `RegSpiller`away from ` VMReg* + int` to `GrowableArray`? - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
On Mon, 9 May 2022 10:28:27 GMT, Jorn Vernee wrote:
>> Hi,
>>
>> This PR updates the VM implementation of the foreign linker, by bringing
>> over commits from the panama-foreign repo.
>>
>> This is split off from the main JEP integration for 19, since we have
>> limited resources to handle this. As such, this PR might fall over to 20.
>>
>> I've written up an overview of the Linker architecture here:
>> http://cr.openjdk.java.net/~jvernee/docs/FL_Overview.html it might be useful
>> to read that first.
>>
>> This patch moves from the "legacy" implementation, to what is currently
>> implemented in the panama-foreign repo, except for replacing the use of
>> method handle combinators with ASM. That will come in a later path. To
>> recap. This PR contains the following changes:
>>
>> 1. VM stubs for downcalls are now generated up front, instead of lazily by
>> C2 [1].
>> 2. the VM support for upcalls/downcalls now support all possible call
>> shapes. And VM stubs and Java code implementing the buffered invocation
>> strategy has been removed [2], [3], [4], [5].
>> 3. The existing C2 intrinsification support for the `linkToNative` method
>> handle linker was no longer needed and has been removed [6] (support might
>> be re-added in another form later).
>> 4. Some other cleanups, such as: OptimizedEntryBlob (for upcalls) now
>> implements RuntimeBlob directly. Binding to java classes has been rewritten
>> to use javaClasses.h/cpp (this wasn't previously possible due to these java
>> classes being in an incubator module) [7], [8], [9].
>>
>> While the patch mostly consists of VM changes, there are also some Java
>> changes to support (2).
>>
>> The original commit structure has been mostly retained, so it might be
>> useful to look at a specific commit, or the corresponding patch in the
>> [panama-foreign](https://github.com/openjdk/panama-foreign/pulls?q=is%3Apr)
>> repo as well. I've also left some inline comments to explain some of the
>> changes, which will hopefully make reviewing easier.
>>
>> Testing: Tier1-4
>>
>> Thanks,
>> Jorn
>>
>> [1]:
>> https://github.com/openjdk/jdk/pull/7959/commits/048b88156814579dca1f70742061ad24942fd358
>> [2]:
>> https://github.com/openjdk/jdk/pull/7959/commits/2fbbef472b4c2b4fee5ede2f18cd81ab61e88f49
>> [3]:
>> https://github.com/openjdk/jdk/pull/7959/commits/8a957a4ed9cc8d1f708ea8777212eb51ab403dc3
>> [4]:
>> https://github.com/openjdk/jdk/pull/7959/commits/35ba1d964f1de4a77345dc58debe0565db4b0ff3
>> [5]:
>> https://github.com/openjdk/jdk/pull/7959/commits/4e72aae22920300c5ffa16fed805b62ed9092120
>> [6]:
>> https://github.com/openjdk/jdk/pull/7959/commits/08e22e1b468c5c8f0cfd7135c72849944068aa7a
>> [7]:
>> https://github.com/openjdk/jdk/pull/7959/commits/451cd9edf54016c182dab21a8b26bd8b609fc062
>> [8]:
>> https://github.com/openjdk/jdk/pull/7959/commits/4c851d2795afafec3a3ab17f4142ee098692068f
>> [9]:
>> https://github.com/openjdk/jdk/pull/7959/commits/d025377799424f31512dca2ffe95491cd5ae22f9
>
> Jorn Vernee has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 21 commits:
>
> - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2
> - Remove unneeded ComputeMoveOrder
> - Remove comment about native calls in lcm.cpp
> - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64
>
>Reviewed-by: jvernee, mcimadamore
> - Update riscv and arm stubs
> - Remove spurious ProblemList change
> - Pass pointer to LogStream
> - Polish
> - Replace TraceNativeInvokers flag with unified logging
> - Fix other platforms, take 2
> - ... and 11 more:
> https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91
Nice work! Looks good. Some minor comments/questions follow.
src/hotspot/cpu/aarch64/frame_aarch64.cpp line 379:
> 377: // need unextended_sp here, since normal sp is wrong for interpreter
> callees
> 378: return reinterpret_cast(
> 379: reinterpret_cast(frame.unextended_sp()) +
> in_bytes(_frame_data_offset));
Maybe use `address` instead of `char*`?
src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5531:
> 5529: }
> 5530:
> 5531: // On 64 bit we will store integer like items to the stack as
Time for a cleanup? `64 bit` vs `64bit`, `abi`, `Aarch64`.
src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5547:
> 5545: } else if (dst.first()->is_stack()) {
> 5546: // reg to stack
> 5547: // Do we really have to sign extend???
Obsolete? Remove?
src/hotspot/cpu/aarch64/universalUpcallHandler_aarch64.cpp line 306:
> 304: intptr_t exception_handler_offset = __ pc() - start;
> 305:
> 306: // Native caller has no idea how to handle exceptions,
Can you elaborate, please, how it is expected to work in presence of
asynchronous exceptions? I'd expect to see a code which unconditionally clears
pending exception with an assertion that verifies that the exception is of
expected type.
src/hotspot/cpu/x86/foreign_globals_x86.
Re: RFR: 8283689: Update the foreign linker VM implementation [v7]
> Hi, > > This PR updates the VM implementation of the foreign linker, by bringing over > commits from the panama-foreign repo. > > This is split off from the main JEP integration for 19, since we have limited > resources to handle this. As such, this PR might fall over to 20. > > I've written up an overview of the Linker architecture here: > http://cr.openjdk.java.net/~jvernee/docs/FL_Overview.html it might be useful > to read that first. > > This patch moves from the "legacy" implementation, to what is currently > implemented in the panama-foreign repo, except for replacing the use of > method handle combinators with ASM. That will come in a later path. To recap. > This PR contains the following changes: > > 1. VM stubs for downcalls are now generated up front, instead of lazily by C2 > [1]. > 2. the VM support for upcalls/downcalls now support all possible call shapes. > And VM stubs and Java code implementing the buffered invocation strategy has > been removed [2], [3], [4], [5]. > 3. The existing C2 intrinsification support for the `linkToNative` method > handle linker was no longer needed and has been removed [6] (support might be > re-added in another form later). > 4. Some other cleanups, such as: OptimizedEntryBlob (for upcalls) now > implements RuntimeBlob directly. Binding to java classes has been rewritten > to use javaClasses.h/cpp (this wasn't previously possible due to these java > classes being in an incubator module) [7], [8], [9]. > > While the patch mostly consists of VM changes, there are also some Java > changes to support (2). > > The original commit structure has been mostly retained, so it might be useful > to look at a specific commit, or the corresponding patch in the > [panama-foreign](https://github.com/openjdk/panama-foreign/pulls?q=is%3Apr) > repo as well. I've also left some inline comments to explain some of the > changes, which will hopefully make reviewing easier. > > Testing: Tier1-4 > > Thanks, > Jorn > > [1]: > https://github.com/openjdk/jdk/pull/7959/commits/048b88156814579dca1f70742061ad24942fd358 > [2]: > https://github.com/openjdk/jdk/pull/7959/commits/2fbbef472b4c2b4fee5ede2f18cd81ab61e88f49 > [3]: > https://github.com/openjdk/jdk/pull/7959/commits/8a957a4ed9cc8d1f708ea8777212eb51ab403dc3 > [4]: > https://github.com/openjdk/jdk/pull/7959/commits/35ba1d964f1de4a77345dc58debe0565db4b0ff3 > [5]: > https://github.com/openjdk/jdk/pull/7959/commits/4e72aae22920300c5ffa16fed805b62ed9092120 > [6]: > https://github.com/openjdk/jdk/pull/7959/commits/08e22e1b468c5c8f0cfd7135c72849944068aa7a > [7]: > https://github.com/openjdk/jdk/pull/7959/commits/451cd9edf54016c182dab21a8b26bd8b609fc062 > [8]: > https://github.com/openjdk/jdk/pull/7959/commits/4c851d2795afafec3a3ab17f4142ee098692068f > [9]: > https://github.com/openjdk/jdk/pull/7959/commits/d025377799424f31512dca2ffe95491cd5ae22f9 Jorn Vernee has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 21 commits: - Merge branch 'foreign-preview-m' into JEP-19-VM-IMPL2 - Remove unneeded ComputeMoveOrder - Remove comment about native calls in lcm.cpp - 8284072: foreign/StdLibTest.java randomly crashes on MacOS/AArch64 Reviewed-by: jvernee, mcimadamore - Update riscv and arm stubs - Remove spurious ProblemList change - Pass pointer to LogStream - Polish - Replace TraceNativeInvokers flag with unified logging - Fix other platforms, take 2 - ... and 11 more: https://git.openjdk.java.net/jdk/compare/3c88a2ef...43fd1b91 - Changes: https://git.openjdk.java.net/jdk/pull/7959/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7959&range=06 Stats: 6934 lines in 157 files changed: 2678 ins; 3218 del; 1038 mod Patch: https://git.openjdk.java.net/jdk/pull/7959.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7959/head:pull/7959 PR: https://git.openjdk.java.net/jdk/pull/7959
