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

[Remi Forax]

ok, i'm tired.
forget my email, the goal of 8223029 is to fix that, as said in the title.

Rémi

----- Mail original -----
> De: "Remi Forax" <forax at univ-mlv.fr>
> À: "Tobias Hartmann" <tobias.hartmann at oracle.com>
> Cc: "valhalla-dev" <valhalla-dev at openjdk.java.net>
> Envoyé: Vendredi 10 Mai 2019 22:05:58
> Objet: Re: RFR(M): 8223029: [lworld] C2 support for widening/narrowing conversion "[QFoo;" <: "[LFoo;”

> Hi Tobias,
> How can you create a [QFoo; inside a [LFoo; given the checkcast fails ?
> 
>  private static final IntBox[] ARRAY = new IntBox[100_000];
>  static {
>    IntStream.range(0, ARRAY.length).forEach(i -> ARRAY[i] = IntBox.valueOf(i));
>    Collections.shuffle(Arrays.asList((IntBox?[])ARRAY));
>  }
> 
> with IntBox an inline class that wraps an int.
> 
> at runtime i get:
>  Caused by: java.lang.ClassCastException: class
>  [Qfr.umlv.rivieradev.lworld.IntBox; cannot be cast to class
>  [Lfr.umlv.rivieradev.lworld.IntBox; ([Qfr.umlv.rivieradev.lworld.IntBox; and
>  [Lfr.umlv.rivieradev.lworld.IntBox; are in module fr.umlv.rivieradev.lworld at 1.0
>  of loader 'app')
> 
> Rémi
> 
> ----- Mail original -----
>> De: "Tobias Hartmann" <tobias.hartmann at oracle.com>
>> À: "valhalla-dev" <valhalla-dev at openjdk.java.net>
>> Envoyé: Vendredi 10 Mai 2019 16:08:27
>> Objet: Re: RFR(M): 8223029: [lworld] C2 support for widening/narrowing
>> conversion "[QFoo;" <: "[LFoo;”
> 
>> 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/