RFR(M): 8222221: [lworld] Valhalla causes performance slowdown for reflective invocations

[Tobias Hartmann]

Hi Roland,

thanks for the review!

On 02.07.19 15:51, Roland Westrelin wrote:
> Why drop stuff related to Compile::_type_hwm?

Because it's unused code.
> Why no check for is_not_flat() for the first generate_valueArray_guard
> below?
Because we only have a TypeKlassPtr from the array type mirror but no TypeAryPtr for the destination
type.

> Shouldn't the is_not_flat() test be in
> generate_valueArray_guard()?
> 
> 4042       Node* is_flat = generate_valueArray_guard(klass_node, NULL);
> 4043       if (!original_t->is_not_flat()) {
> 4044         generate_valueArray_guard(original_kls, bailout);
> 4045       }

generate_valueArray_guard takes a klass but the is_not_flat() check needs to be done on TypeAryPtr.

> In Parse::array_store(), why remove:
> 
> || elemptr->is_valuetypeptr()
> 
> ?

I don't think it's necessary, right? Either it is a null-free array and then
!ary_t->klass_is_exact() holds or it is a null-ok array and then we don't need to go this path.

> 253         Node *kls = load_object_klass(ary);
> 254         Node *layout_val = load_lh_array_tag(kls);
> 
> Isn't the usual coding convention Node* kls?

It surely is, not sure why this got changed. Fixed.

Added a missing call to remove_speculative() in parse1.cpp that caused a typesystem assert because
't' could already contain a speculative type.

New webrev:
http://cr.openjdk.java.net/~thartmann/8222221/webrev.01

Thanks,
Tobias