[lworld] RFR: lworld aaload/aastore

[Karen Kinnear]

Mr Simms,

Thank you so much for implementing this and clearly describing the JVMS changes and behavior we agreed to
prototype. And I am grateful for all the test cases and experiments with array behavior with
existing classes.

The code looks good. (If you already pushed it great - I didn’t see a notification).

Minor questions/comments
1. interpreterRuntime.cpp 496 - value_array_load - you changed the assertion check, so the message no longer matches

2. templateTable_x86.cpp
There are two places that say // Move superclass into pax - but I think they are moving the element class?
lines 1260 and 1194

3. So my understanding is that: all arrays with a value type element are flattenable
and the flatten decision is made in ValueKlass::array_klass_impl, so you either get a ValueArrayKlass - always
flattened or an ObjArrayKlass with element type value type - so keeps non-nullable semantics.
Is that accurate?

So test_flat_array_oop is a flattened test and all ValueTypeArrays set the vt_array bit.

looking forward to the profiling info.

thanks!
Karen

> On Mar 21, 2018, at 9:28 AM, David Simms <david.simms at oracle.com> wrote:
> 
> 
> Updated version of aaload/aastore and copy_array for the interpreter...
> 
> http://cr.openjdk.java.net/~dsimms/valhalla/lworld_value_aaloadstore/webrev3/
> 
> Assumes the current thinking that all value type arrays are flat, but are covariant with Object array and interface arrays:
> 
> * aaload/aastore: works with both objArrayOop and valueArrayOop (flat)
>     o currently assumes always flat semantics for value type element
>       class even for objArrayOop (if value deemed unsuitable as flat),
>       i.e. no null-ability
>     o objArrayOop with values works as vanilla array, i.e. has
>       null-ability, for non-value type classes
> * aaload JVMS changes:
>     o If source array element type is value class (flat array
>       semantics), the result is never null, but equivalent to the
>       "defaultvalue" bytecode.
> * aastore JVMS changes:
>     o May NPE if the target array element type is value class (flat
>       array semantics)
>     o May OOM if the source value is "buffered" and target array is
>       element type is not value class (ref array semantics)
> * covariance
>     o V[] <: I[]: Object[] <: Object where V = value type class, and I
>       = any one of V's interface super types.
>         + E.g. "__ByValue Point implements Comparable", therefore:
>           Point[] <: Comparable[]
>         + instanceof/checkcast, functions as described above
>         + System.arraycopy() (interpreter implementation) now accepts
>           as described above
>             # May NPE when attempting to store null into an array with
>               element type of value class (flat array semantics)
> 
> 
> 
> There are number of tests added to "ValueTypeArray.java", including some sanity test of some "java.util" array related classes. Collections are a larger topic which the test does not try to address fully. But the sanity tests do raise a few interesting observations when fitting value arrays into current JDK collections:
> 
> * ArrayList and ArrayDeque specifically use "Object[] elementData",
>   and accepts no subtype (see "ArrayList(Collection<? extends E> c)",
>   will even copy the source array if not Object[])
>     o So given System.arraycopy from value array to object array now
>       works, as does aaload/store of a value to and from Object array,
>       it all works (in so far as the sanity check tests). Even
>       ArrayList.remove(), since the backing store is always a
>       reference array.
> * Arrays.toList() produces its own ArrayList with a backing array of
>   the sharp type, so potentially a value type array without
>   null-ability characteristics
>     o ...this is fine, doesn't implement "remove()".
> 
> Given what I said about ArrayList backing being Object[] ("Collection.toArray()" must return component type Object), and the API allowing nulls, our current javac behavior is a little weird. Check this code out:
> 
>     ArrayList<MyInt> aList = new ArrayList<MyInt>(Arrays.asList(copyMyInts));
> 
>     aList.add((MyInt)getNull()); // <<< Some issues here, should this be legal ? With "ArrayList<>", yes but maybe not with "ArrayList<Value-Type>"
> 
> Currently javac is inserting "java/util/Objects.requireNonNull". At first I wanted to argue that check-cast should handle null with value class automatically. But wait, generic erasure means I see "ArrayList.add(LObject;)" in the VM, so null is perfectly fine...but the intention is "ArrayList<MyInt>.add(LMyInt;)...so my bad ?
> 
> 
> /D
> 
>