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

Reply via email to