RFR(S): fix System.arraycopy() C2 intrinsics with arrays of value types

[Tobias Hartmann]

Hi Roland,

your changes look good to me. Here are some comments/questions:

arraycopynode.cpp:
- lines 253, 257: I don't think this is necessary because src_elem/dest_elem are only used in line 261 and you can
replace the check in line 272 by "ary_src->klass()->is_value_array_klass()"
- line 273: the is_aryptr() can be removed

compile.cpp:
- Thanks for removing the too strong asserts. They've triggered failures in our testing.

library_call.cpp:
- I would rename NotArray -> NonArray and NotObjectArray -> NonObjectArray

macroArrayCopy.cpp
- line 329: maybe put the comment in /* */ to avoid ") {" in new line

type.cpp:
- Could you explain why these changes are necessary? In line 4773, how can offset be < 0?

valuetypenode.cpp
- line 323: Why is this required?

TestArrays.java:
- Should we add -XX:-ReduceInitialCardMarks to one of the runs?

Please close 8183920 once you've pushed this.

Thanks,
Tobias

On 16.11.2017 23:13, Roland Westrelin wrote:
> 
> http://cr.openjdk.java.net/~roland/valhalla/arraycopy/webrev.00/
> 
> This changes fixes arraycopy intrinsics (which cover not only
> System.arraycopy() but also clone() and copyOf()) so they work with
> array of value types, flattened or not.
> 
> arraycopy intrinsics rely on stubs and the correct stub must be picked
> for value types. In particular, if the array is flattened, the value
> type has object fields and barriers needs to be emitted on object
> stores, none of the stubs are safe to use and the code should fall back
> to a runtime call.
> 
> Short array copies are optimized as a series of load/stores. Flattened
> arrays need to copy each field of each element of the array. This is
> achieved by creating a GraphKit in ArrayCopyNode::Ideal(). Thanks to
> that, we can now emit gc barriers in Ideal() and optimize previously
> impossible to optimize copies.
> 
> I also verified that:
> 
> - ArrayCopyNode is properly handled by escape analysis (if the
> destination of the copy doesn't escape, an arraycopy doesn't cause it to
> be viewed as escaping).
> 
> - a LoadNode optimization that replaces a load from the destination of a
> arraycopy by a load from the source (in order to optimize the ArrayCopyNode
> out) works
> 
> - when an ArrayCopyNode is eliminated because of a non escaping
> allocation which is scalarized, element values are properly recorded at
> safepoints.
> 
> Roland.
>