[foreign] RFR 8218144: Direct invoker does not handle varargs correctly
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Thu Jan 31 14:10:40 UTC 2019
Hi,
our CI systems detected a spurious failures on StdLibTest, only on Linux
x64. After a lot of digging, I figured out the problem was that SystemV
ABI requires the RAX register to be set to the number of vector
arguments in a variadic call:
%rax: temporary register;with variable arguments passes information
about the number of vector registers used
So, if we are doing a variadic call, we need to set this register to the
number of floating point args we're passing. The universal invoker seems
to do that:
http://hg.openjdk.java.net/panama/dev/file/tip/src/hotspot/cpu/x86/universalNativeInvoker_x86.cpp#l515
For both variadic and non-variadic calls.
So, a natural fix for the direct approach is to tweak the cast to the
function pointers in our native stubs - e.g. from this:
void DNI_invokeNative_V_JD(JNIEnv *env, jobject _unused, jlong addr,
jlong arg0, jdouble arg1) {
((void (*)(jlong, jdouble))addr)(arg0, arg1);
}
To this:
void DNI_invokeNative_V_JD(JNIEnv *env, jobject _unused, jlong addr,
jlong arg0, jdouble arg1) {
((void (*)(jlong, jdouble,...))addr)(arg0, arg1);
}
Which will generate the right set for RAX (even though it will do so
also for non-variadic call - but that's the same as what universal
invoker does).
I also strengthened the test - I tried first to use sprintf instead of
printf, but that didn't work, as that doesn't seem to reproduce the
issue (my suspicion here is that 'printf' is optimized to look into
RAX). So, instead, I tweaked the test to run for a lot longer, by
repeatedly shuffling the permutation array, and I also increased the
decimal length of the double formatting string - that's because,
unfortunately, we were using 1.23, which has 4 chars, and sometimes the
result of printf ended up with "-NaN" which also has 4 chars, despite
being very wrong. With both measures, tests are much more likely, so
much so that in the affected machine I've never been able to get a pass
w/o the proposed patch.
I verified that the patch works on all platforms, and I also verified
that the patch does not introduce performance regressions, which is not
the case (kind of expected, given that the only difference for adding a
"..." is the extra MOV to RAX, as shown here [1]).
Webrev:
http://cr.openjdk.java.net/~mcimadamore/panama/8218144/
Cheers
Maurizio
[1] - https://gcc.godbolt.org/z/VAgfZc
More information about the panama-dev
mailing list