Hi Chris, I know how JNI is meant. However, C/C++ is (almost) never platform independent. Especially when it comes to primitive types. My change is not particularly beautiful, but I haven’t found a more beautiful way to fix it.
Note that SetLongArrayRegion seems to work without the alignment requirement in the product build. However, word tearing could possibly be observed. It's not possible to guarantee element-wise atomicity without alignment because of processor architecture. That’s why I think the assertion makes sense and violations at least in the code which is part of OpenJDK should be fixed IMHO. I had already asked for alternative fixes when I was working on JDK-8220348 (like force the compiler to 64-bit align 64-bit types on stack), but nobody has found a way to do this. Best regards, Martin From: Chris Plummer <chris.plum...@oracle.com> Sent: Dienstag, 25. Februar 2020 18:03 To: Doerr, Martin <martin.do...@sap.com>; OpenJDK Serviceability <serviceability-dev@openjdk.java.net>; hotspot-runtime-dev <hotspot-runtime-...@openjdk.java.net> Subject: Re: RFR(XS): 8239856: [ntintel] asserts about copying unaligned array element [Adding runtime-dev as this regards the JNI spec] Hi Martin, JNI is meant as a means to write code that interfaces with the JVM in a platform independent way. Therefore the declaration of a jlong or a jdouble should not require any extra platform dependent considerations. This also means requirements of an internal JVM API should not impose any extra requirements on the JNI code. IMHO this should be fixed in hotspot. Maybe fixing it in jni_md.h (if there is a way to force 64-bit alignment) or in the makefiles (force the compiler to 64-bit align) would also be acceptable. thanks, Chris On 2/25/20 3:22 AM, Doerr, Martin wrote: Hi Chris, according to arraycopy.hpp, “arraycopy operations are implicitly atomic on each array element.” This requires 8 Byte alignment for jlong and jdouble. I don’t want to give up this property just because Windows 32 bit doesn’t align them this way by default. All other supported platforms do it right by default. Best regards, Martin From: Chris Plummer <chris.plum...@oracle.com><mailto:chris.plum...@oracle.com> Sent: Montag, 24. Februar 2020 21:52 To: Doerr, Martin <martin.do...@sap.com><mailto:martin.do...@sap.com>; OpenJDK Serviceability <serviceability-dev@openjdk.java.net><mailto:serviceability-dev@openjdk.java.net> Subject: Re: RFR(XS): 8239856: [ntintel] asserts about copying unaligned array element Hi Martin, I'm not so sure I agree with the approach to this fix, nor for the one already done for JDK-8220348. Shouldn't a user be expected to be able to pass a jlong variable to SetLongArrayRegion() without the need for any special platform dependent modifiers added to the declaration of the variable? cheers, Chris On 2/24/20 5:51 AM, Doerr, Martin wrote: Hi, reposting on serviceability-dev (was core-libs-dev before). Bug: https://bugs.openjdk.java.net/browse/JDK-8239856 Webrev: http://cr.openjdk.java.net/~mdoerr/8239856_win32_long_double_align/webrev.00/ Thanks for the review, Thomas! Best regards, Martin From: Thomas Stüfe <thomas.stu...@gmail.com><mailto:thomas.stu...@gmail.com> Sent: Montag, 24. Februar 2020 14:41 To: Doerr, Martin <martin.do...@sap.com><mailto:martin.do...@sap.com> Cc: core-libs-...@openjdk.java.net<mailto:core-libs-...@openjdk.java.net>; Lindenmaier, Goetz <goetz.lindenma...@sap.com><mailto:goetz.lindenma...@sap.com>; Langer, Christoph <christoph.lan...@sap.com><mailto:christoph.lan...@sap.com> Subject: Re: RFR(XS): 8239856: [ntintel] asserts about copying unaligned array element Oh okay. Then it looks okay to me. Cheers, Thomas On Mon, Feb 24, 2020 at 12:56 PM Doerr, Martin <martin.do...@sap.com<mailto:martin.do...@sap.com>> wrote: Hi Thomas, thanks for the quick review. ATTRIBUTE_ALIGNED is defined in hotspot. I can’t use it for src/jdk.jdwp.agent/share/native/libjdwp/ArrayReferenceImpl.c. Christoph had already suggested to make it available for core libs, too, but I haven’t found a good place for it. Best regards, Martin From: Thomas Stüfe <thomas.stu...@gmail.com<mailto:thomas.stu...@gmail.com>> Sent: Montag, 24. Februar 2020 12:52 To: Doerr, Martin <martin.do...@sap.com<mailto:martin.do...@sap.com>> Cc: core-libs-...@openjdk.java.net<mailto:core-libs-...@openjdk.java.net>; Lindenmaier, Goetz <goetz.lindenma...@sap.com<mailto:goetz.lindenma...@sap.com>>; Langer, Christoph <christoph.lan...@sap.com<mailto:christoph.lan...@sap.com>> Subject: Re: RFR(XS): 8239856: [ntintel] asserts about copying unaligned array element Hi Martin, maybe use ATTRIBUTE_ALIGNED instead? Cheers, Thomas On Mon, Feb 24, 2020 at 12:44 PM Doerr, Martin <martin.do...@sap.com<mailto:martin.do...@sap.com>> wrote: Hi, we had fixed stack array alignment for Windows 32 bit with JDK-8220348. However, there are also stack allocated jlong and jdouble used as source for SetLongArrayRegion and SetDoubleArrayRegion with insufficient alignment for this platform. Here’s my proposed fix: http://cr.openjdk.java.net/~mdoerr/8239856_win32_long_double_align/webrev.00/ Please review. Best regards, Martin
