Re: RFR(M): 8223029: [lworld] C2 support for widening/narrowing conversion "[QFoo;" <: "[LFoo;”

[Tobias Hartmann]

Hi,

while performing some extended testing, I found an issue:
We should not optimize subtype checking to be performed on the element klasses for [V? arrays. The
runtime type might be [V due to [V <: [V? and the klass for [V? and [V is the same but the component
mirror is not (it's either the ValType or the BoxType). I've changed the implementation of
Compile::static_subtype_check to perform a full test in this case and disabled optimizations for the
full check in subnode.cpp.

I've also implemented folding of component and value mirror loads and had to change the value mirror
load types because the result can be null for non-value types.

IR verification now triggers for test28. I've disabled it for now and will file a follow up bug.

Incremental webrev:
http://cr.openjdk.java.net/~thartmann/8223029/webrev.inc/

Full webrev:
http://cr.openjdk.java.net/~thartmann/8223029/webrev.01/

All tests pass.

Thanks,
Tobias

On 08.05.19 14:00, Tobias Hartmann wrote:
> Hi,
> 
> please review the following patch:
> https://bugs.openjdk.java.net/browse/JDK-8223029
> http://cr.openjdk.java.net/~thartmann/8223029/webrev.00/
> 
> This is based on Mr. Simms' patch for 8223017 [1].
> 
> I've changed the type system (ciTypeFlow.cpp and type.cpp) to not fall back to [Object when meeting
> [V and [V? because [V <: [V? allows conversion between the two. Whenever we access a [V?, we now
> need to check if the actual type is a (potentially flattened) [V. I've changed the array_load and
> array_store implementation in parse2.cpp accordingly. In addition, I disabled folding of klass loads
> for [V? in memnode.cpp because the actual type might be [V.
> 
> I've noticed that GraphKit::gen_value_type_array_guard() currently always deoptimizes when storing
> null to a [V? because we only check if the array element is a value type but not if the array is
> really null-free. I've fixed that and added a corresponding test (TestArrayAccessDeopt.java).
> 
> I've also noticed that sometimes we were only checking ValueArrayFlatten where we should really also
> check vk->flatten_array() if the klass is known. I've fixed that as well and added a test
> (TestFlatArrayThreshold.java)
> 
> I've added lots of tests to TestNullableArrays to verify correctness of widening and narrowing
> conversions.
> 
> Changes to other files are refactoring.
> 
> Thanks,
> Tobias
> 
> [1] http://cr.openjdk.java.net/~dsimms/valhalla/8223017/
>