Coleen,
Thank you for addressing this!
On 2017-10-25 18:49, coleen.phillim...@oracle.com wrote:
Summary: removed hotspot version of jvm*h and jni*h files
Mostly used sed to remove prims/jvm.h and move #include "jvm.h" after
precompiled.h, so if you have repetitive stress wrist issues don't
click on most of these files.
There were more issues to resolve, however. The JDK windows jni_md.h
file defined jint as long and the hotspot windows jni_x86.h as int. I
had to choose the jdk version since it's the public version, so there
are changes to the hotspot files for this. Generally I changed the
code to use 'int' rather than 'jint' where the surrounding API didn't
insist on consistently using java types. We should mostly be using C++
types within hotspot except in interfaces to native/JNI code. There
are a couple of hacks in places where adding multiple jint casts was
too painful.
Tested with JPRT and tier2-4 (in progress).
open webrev at http://cr.openjdk.java.net/~coleenp/8189610.01/webrev
Looks great!
Just a few comments:
* src/java.base/unix/native/include/jni_md.h:
I don't think the externally_visible attribute should be there for arm.
I know this was the case for the corresponding hotspot file for arm, but
that was techically incorrect. The proper dependency here is that
externally_visible should be in all JNIEXPORT if and only if we're
building with JVM feature "link-time-opt". Traditionally, that feature
been enabled when building arm32 builds, and only then, so there's been
a (coincidentally) connection here. Nowadays, Oracle does not care about
the arm32 builds, and I'm not sure if anyone else is building them with
link-time-opt enabled.
It does seem wrong to me to export this behavior in the public jni_md.h
file, though. I think the correct way to solve this, if we should
continue supporting link-time-opt is to make sure this attribute is set
for exported hotspot functions. If it's still needed, that is. A quick
googling seems to indicate that visibility("default") might be enough in
modern gcc's.
A third option is to remove the support for link-time-opt entirely, if
it's not really used.
* src/java.base/unix/native/include/jvm_md.h and
src/java.base/windows/native/include/jvm_md.h:
These files define a public API, and contain non-trivial changes. I
suspect you should file a CSR request. (Even though I realize you're
only matching the header file with the reality.)
/Magnus
bug link https://bugs.openjdk.java.net/browse/JDK-8189610
I have a script to update copyright files on commit.
Thanks to Magnus and ErikJ for the makefile changes.
Thanks,
Coleen
