RFR: 8328480: C2: SubTypeCheckNode in checkcast should use the klass constant of a unique concrete sub class

[Christian Hagedorn]

On Wed, 27 Mar 2024 16:47:05 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> I was surprised by that as well when I've run testing but `gen_checkcast()` is actually also called in `Parse::array_store_check()` which passes a `LoadKlassNode` which is not a constant:
>> 
>> https://github.com/openjdk/jdk/blob/05854fd704cba6ebd73007d9547a064891d49587/src/hotspot/share/opto/parseHelper.cpp#L233-L238
>> 
>> It states that it ignores the result and just calls it for the CFG effects - does not sound like a very clean solution.
>
> I'm confused by this. Looking at `TypeInstKlassPtr::try_improve()` returns a constant only if it finds a unique subklass and that subklass is final. So it should not be uncommon to have an `improved_klass_ptr_type` that's not constant.

That's true. I've missed that we could also improve non-constants like `LoadKlass`, for example. I'm not sure if we could also have other improved types here apart from constants or `LoadKlass` (from `array_store_check()`). The only other non-constant `superklass` is passed by `LibraryCallKit::inline_Class_cast()`. From there, we either get a constant or a `CastPPNode` from which I'm not sure if it can really be improved.

Either way, I think we could improve the code like that to get an improved type regardless of the node, what do you think?


    if (improved_klass_ptr_type != klass_ptr_type) {
      if (improved_klass_ptr_type->singleton()) {
        improved_superklass = makecon(improved_klass_ptr_type);
      } else {
        superklass->raise_bottom_type(improved_klass_ptr_type);
        _gvn.set_type(superklass, improved_klass_ptr_type);
      }
    }

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18515#discussion_r1543011083