[lworld] RFR: lworld aaload/aastore
David Simms
david.simms at oracle.com
Wed Mar 14 14:41:32 UTC 2018
Thanks for the heads up...I will double check with my latest fixes +
latest repo.
Since posting the RFR yesterday I've increased tests and found a few
bugs. Also started implementing covariance and arraycopy to handle it...
We'll see if I can update the webrev today or early tomorrow.
/D
On 14/03/18 14:21, Tobias Hartmann wrote:
> Update: All tests pass with -XX:-ProfileInterpreter -Xint.
>
>
> On 14.03.2018 14:17, Tobias Hartmann wrote:
>> Hi David,
>>
>> I've executed test/hotspot/jtreg/compiler/valhalla/valuetypes/TestArrays.java with -Xint and your
>> patch but I'm seeing crashes:
>>
>> # Internal Error
>> (/oracle/valhallaL/open/src/hotspot/share/interpreter/interpreterRuntime.cpp:1470), pid=8211, tid=8212
>> # assert(mdp == mdp2) failed: wrong mdp
>>
>> Stack: [0x00007fa7ea72f000,0x00007fa7ea830000], sp=0x00007fa7ea82da10, free space=1018k
>> Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native
>> code)
>> V [libjvm.so+0x1990cec] VMError::report_and_die(int, char const*, char const*, __va_list_tag*,
>> Thread*, unsigned char*, void*, void*, char const*, int, unsigned long)+0x18c
>> V [libjvm.so+0x1991b1f] VMError::report_and_die(Thread*, char const*, int, char const*, char
>> const*, __va_list_tag*)+0x2f
>> V [libjvm.so+0xba33a2] report_vm_error(char const*, int, char const*, char const*, ...)+0x112
>> V [libjvm.so+0xf7a86f] InterpreterRuntime::verify_mdp(Method*, unsigned char*, unsigned char*)+0x2df
>> j compiler.valhalla.valuetypes.TestArrays.test15()I+35
>> j compiler.valhalla.valuetypes.TestArrays.test15_verifier(Z)V+1
>> v ~StubRoutines::call_stub
>> V [libjvm.so+0xfaf6d9] JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*,
>> Thread*)+0x819
>> V [libjvm.so+0x16fade8] invoke(InstanceKlass*, methodHandle const&, Handle, bool, objArrayHandle,
>> BasicType, objArrayHandle, bool, Thread*)+0xc18
>> V [libjvm.so+0x16ffeab] Reflection::invoke_method(oop, Handle, objArrayHandle, Thread*)+0x1cb
>> V [libjvm.so+0x1135978] JVM_InvokeMethod+0x228
>> j
>> jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Ljava/lang/reflect/Method;Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;+0
>> java.base
>> j
>> jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;+100
>> java.base
>> j
>> jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;+6
>> java.base
>> j java.lang.reflect.Method.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;+59 java.base
>> j compiler.valhalla.valuetypes.ValueTypeTest.run([Ljava/lang/Class;)V+237
>> j compiler.valhalla.valuetypes.ValueTypeTest.run([Ljava/lang/String;[Ljava/lang/Class;)V+14
>> j compiler.valhalla.valuetypes.TestArrays.main([Ljava/lang/String;)V+29
>> v ~StubRoutines::call_stub
>> V [libjvm.so+0xfaf6d9] JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*,
>> Thread*)+0x819
>> V [libjvm.so+0x108cced] jni_invoke_static(JNIEnv_*, JavaValue*, _jobject*, JNICallType,
>> _jmethodID*, JNI_ArgumentPusher*, Thread*) [clone .isra.168]+0x39d
>> V [libjvm.so+0x10aacb5] jni_CallStaticVoidMethod+0x205
>> C [libjli.so+0x4687] JavaMain+0xa57
>>
>> Thanks,
>> Tobias
>>
>>
>> On 13.03.2018 17:32, David Simms wrote:
>>> Hi,
>>>
>>> Here's a patch to implement the new lworld semantics for aaload and aastore with values:
>>>
>>> http://cr.openjdk.java.net/~dsimms/valhalla/lworld_value_aaloadstore/webrev0/
>>>
>>> Hotspot implementation currently allows value to be contained in one of three types of arrays, and
>>> the aaload/aastore differs slightly depending on the array type:
>>>
>>> * "Flat value array": valueArrayOop, specifically for holding values
>>> of "element_klass" type (always a "value type") and nothing else
>>> o Loads and stores of null are not permitted
>>> * "Object array": objArrayOop whose "element_klass" is not a "value
>>> type", e.g. "Object", "Comparable" etc
>>> o Maintains the same behavior for both objects and values.
>>> * "Non-flat value array": objArrayOop whose "element_klass" is
>>> specifically a "value type". This is left in the code for handling
>>> special cases like "atomic value class" or "really large value
>>> class" or "many embedded oops value class", where flattening seems
>>> unsuitable. Unclear if this use-case will survive in the long run.
>>> o Maintains the same semantics as "Flat Value Array", i.e.:
>>> Loads and stores of null are not permitted
>>>
>>> So semantically, there are two types, "value array" and "object array".
>>>
>>> Testing: "runtime/valhalla/valuetypes/ValueTypeArray.java" now passes. I was a bit concerned for
>>> "aaload" performance given it needs to test for flat array oop (i.e. loads "klass->_layout_helper").
>>> I added a simulation in a patch in the experiment ("exp") branch, and through it through our
>>> performance "sanity" benchmarks, no regressions. Even small micro-benchmark had a hard time seeing
>>> any difference. So I will avoid any premature optimization for now, until proven otherwise.
>>>
>>> TODO: some further test cases which are better suited to ValueOops and bytecode API testing, which
>>> is a different bug. Also, for the objArrayOop containers I didn't bother passing by value, so a
>>> further test for ensuring "withfield" is COW op and not affecting the reference would be a good idea.
>>>
>>> /D
>>>
More information about the valhalla-dev
mailing list