Unification of jni_<platform>.h

[Doerr, Martin]

Hi Coleen,

sure, we can test it when a webrev is available.

Best regards,
Martin


-----Original Message-----
From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of coleen.phillimore at oracle.com
Sent: Montag, 23. Oktober 2017 14:11
To: hotspot-runtime-dev at openjdk.java.net
Subject: Re: Unification of jni_<platform>.h


With my change that I'm working on for JDK-8189610, I'm cleaning up the 
jni_platform files also.    They were mostly the same with some minor 
differences that we're working through.

Martin, when I get this working here, can you test out your platforms on it?

thanks,
Coleen

On 10/23/17 5:33 AM, Doerr, Martin wrote:
> Hi,
>
> we only support 64 bit on s390. Seems like the code could be cleaned up or updated. Thanks for looking into it.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of David Holmes
> Sent: Montag, 23. Oktober 2017 05:47
> To: Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com>; hotspot-runtime-dev at openjdk.java.net; build-dev <build-dev at openjdk.java.net>
> Subject: Re: Unification of jni_<platform>.h
>
> Hi Magnus,
>
> On 18/10/2017 11:10 PM, Magnus Ihse Bursie wrote:
>> In the context of JDK-8167078 (Duplicate header files in hotspot and
>> jdk), I was looking at unifying the platform specific extensions to
>> jni.h, the jni_md.h files between hotspot and java.base.
>>
>> The main difference here is that the hotspot jni_* files are divided
>> into individual files based on cpu, while the java.base versions are
>> divided according to operating system (which, in turn, implicates
>> compiler). On a second look, however, it turns out to not be so
>> problematic -- a hotspot file like jni_x86.h contains basically
>> #ifdef WIN32
>> ... // do windows stuff
>> #else
>> ... // do posix/macos stuff
>> #endif
>>
>> and the two blocks match very well what the java.base versions are doing
>> for the different operating systems.
>>
>> In fact, the OS (and compiler) division seems more natural, since there
>> is much redundancy in the hotspot files, and the java.base OS-based
>> versions are much simpler.
>>
>> I'm proposing to remove the hotspot CPU-based files and just replace
>> them with the java.base versions. However, a few differences crept out.
>> I'd like to discuss them before proceeding.
>>
>> For jni_aarch64.h: There's a windows (or rather, "not unix") part of the
>> jni_aarch64.h that I do not believe have ever been used. Nevertheless,
>> it contains "typedef int jint" rather than "typedef long jint", which
>> the java.base windows version uses. Since I've never heard of aarch64
>> building on Windows, I presume this is irrelevant.
> Likely just copied from the x86 version at some point. There is no
> Windows-aarch64 support in the OpenJDK.
>
>> For jni_aarch64.h and jni_sparc.h: The unix version will match with the
>> java.base version as long as _LP64 is defined. This should be just fine,
>> since we define that when building hotspot/the JDK, and the java.base
>> version that is exported by the JDK on linux/aarch64 and solaris/sparc
>> has always required the _LP64 define.
> Yep 64-bit only.
>
>> (In fact, the macos and unix versions in java.base only contain trivial
>> differences. The macos version should probably be removed in favor of a
>> single unix version.)
>>
>> For jni_x86.h: There windows part suffers the same problem as for
>> aarch64, but here it's potentially problematic. The hotspot version uses
>> "typedef int jint" while the java.base version uses "typedef long jint".
>> If this *really* were a problem, we would probably have gotten reports
>> from JNI code unable to work on Windows, so I assume this turns out to
>> be the same datatype in the end. Don't know if it's due to luck,
>> compiler flags or by definition.
> Windows is either ILP32 or LLP64 - in both cases int and long are the
> same and 32-bits.
>
>> For jni_s390.h: Here's the most problematic version. The exported
>> java.base version uses:
>> #ifdef _LP64
>> typedef long jlong;
>> #else
>> typedef long long jlong;
>> #endif
>> but s390 uses:
>> typedef long int jlong;
>>
>> My best assumption here is that we're only really using s390x (the
>> 64-bit version) and that "long int" is functionally equivalent to
>> "long". Or that jni is broken on s390 and nobody really noticed.
>>
>> Also, s390 uses a simplified version of the gcc attribute dance used for
>> JNIEXPORT/JNIIMPORT. I think the s390 version is just not really
>> updated, and that using the newer in java.base is harmless.
> Can't comment on s390.
>
>> For jni_arm.h: Finally, the jni_arm.h (the 32-bit formerly closed Oracle
>> port), the JNIEXPORT/JNIIMPORT is different, by defining
>> __attribute__((externally_visible)). This might have been relevant due
>> to compile/link time symbol processing for that platform, but
>> technically it should probably not have been connected to the platform
>> per se, but rather to the compilation procedure. Since the arm-32
>> platform is kind of abandoned right now, I propose to modify the
>> compilation to be more standard, if this is required, rather than
>> keeping these attributes.
> As Dean stated this likely related to LTO. And unfortunately attention
> was not paid to the comment:
>
> // Note: please do not change these without also changing jni_md.h in
> the JDK
> // repository
>
> possibly due to open/closed issues back in time. But I'd say we would
> want the hotspot version so that nothing that would previously work is
> now broken.
>
> Thanks,
> David
>
>> /Magnus