On Mon, 23 Nov 2020 18:58:15 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 with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains seven additional > commits since the last revision: > > - Merge branch 'master' into Linker_32bit-fixes_New-Master > - - Add negative test for 32-bit platform. > - Added note to CLinker about failure to initialize on unsupported > platforms > - Remove UnsupportedPlatform test > - Remove unneeded cast > - Remove Stuff that makes the jdk_foreign tests pass > - fix test warnings > - - Fix 32-bit build errors and tests > - Add negative test for 32-bit platform. > - Change CABI to fail more lazily when running on an unsupported platform. > - Change CLinker layouts to be null on unsupported platforms, instead of > failing when initializing the class > - Added note to CLinker about failure to initialize on unsupported > platforms Some minor comments before looking more closely. src/hotspot/cpu/x86/universalNativeInvoker_x86.cpp line 132: > 130: __ ret(0); > 131: #else > 132: __ hlt(); // NYI "Not Implemented Yet" is "NIY". But then there is a macro you can use here: `Unimplemented()`. src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/PlatformLayouts.java line 144: > 142: * The {@code T*} native type. > 143: */ > 144: public static final ValueLayout C_POINTER = > ofPointer(LITTLE_ENDIAN, 64); I somewhat get the change in `Win64()` and `AArch64()`, but why here at `SysV()`? Surely x86_32 is the platform with 32-bit pointers? ------------- PR: https://git.openjdk.java.net/jdk/pull/1266