[lworld] RFR: lworld aaload/aastore

[Tobias Hartmann]

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
>