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

[Remi Forax]

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/