[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