On Mon, 23 Nov 2020 20:36:11 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> JDK-8254231 breaks the Linux and Windows x86 (32-bit) builds. This contains
>> the needed changes to get it working again.
>>
>> Perhaps the most interesting change is adding the `JNI_ENTRY_CPP_NOENV`
>> macro. Using just JNI_ENTRY was causing a linkage failure, due to the
>> declaration of the function in the class not having the same linkage
>> specifiers. It looks like we can't just specify C linkage for class member
>> functions.
>>
>> However, in this case C linkage is not required, so I've added the new macro
>> which doesn't have `extern "C"`. I've also dropped the `JNIEnv*` parameter,
>> since it was not being used, but causing extra work for the caller.
>>
>> Other than that, it's just about adding default definitions for missing
>> functions, and moving around some code in MacroAssembler to be in the
>> correct `#ifdef` blocks.
>>
>> Testing: `make images` on Linux and Windows x86_32 platforms.
>
> Jorn Vernee has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Use the Unimplemented() macro instead of hlt()
I have a few minor comments, in addition to what David is saying about
`JNI_ENTRY_CPP_NOENV`.
src/hotspot/cpu/x86/macroAssembler_x86.cpp line 497:
> 495: void MacroAssembler::reset_last_Java_frame(bool clear_fp) {
> 496: reset_last_Java_frame(r15_thread, clear_fp);
> 497: }
I see that before the Foreign Linker integration, this method was between
`MacroAssembler::pushptr` and `MacroAssembler::set_last_Java_frame`. Please
move it back there.
-------------
Changes requested by shade (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1266
