RFR: 8370405: C2: mismatched store from MergeStores wrongly scalarized in allocation elimination [v2]
Quan Anh Mai
qamai at openjdk.org
Wed Oct 29 17:07:44 UTC 2025
On Wed, 29 Oct 2025 17:00:08 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:
>> Then we come here:
>>
>> 3355 // Generate the subtype check
>> 3356 Node* improved_superklass = superklass;
>> 3357 if (improved_klass_ptr_type != klass_ptr_type && improved_klass_ptr_type->singleton()) {
>> 3358 improved_superklass = makecon(improved_klass_ptr_type);
>> 3359 }
>> 3360 Node* not_subtype_ctrl = gen_subtype_check(not_null_obj, improved_superklass);
>> 3361
>> 3362 // Plug in success path into the merge
>> 3363 cast_obj = _gvn.transform(new CheckCastPPNode(control(), not_null_obj, toop));
>>
>> This gives us a `cast_obj` that knows about `Enumeration`:
>>
>> 350 CheckCastPP === 348 335 [[ ]] #java/lang/Object (java/util/Enumeration):NotNull * Oop:java/lang/Object (java/util/Enumeration):NotNull * !jvms: ClassLoader::getResources @ bci:45 (line 1445)
>>
>> So things are looking promising for now, `res` is that `350 CheckCastPP`. But out in `array_store`, this is not what gets picked up when we do: `val = pop();`.
>> Instead we get the null-check only `335 CastPP`. So somehow it must have been lost?
>> Tracking the slot, I see that `Parse::array_store_check` does `replace_in_map(value, cast);`.
>> But we don't seem to do that for `GraphKit::gen_checkcast`.
>>
>> @merykitty @rwestrel Maybe we should investigate this separately from this bugfix here?
>
> Yes I agree that we should investigate this separately.
We have these lines:
// Note I do NOT always 'replace_in_map(obj,result)' here.
// if( tk->klass()->can_be_primary_super() )
// This means that if I successfully store an Object into an array-of-String
// I 'forget' that the Object is really now known to be a String. I have to
// do this because we don't have true union types for interfaces - if I store
// a Baz into an array-of-Interface and then tell the optimizer it's an
// Interface, I forget that it's also a Baz and cannot do Baz-like field
// references to it. FIX THIS WHEN UNION TYPES APPEAR!
// replace_in_map( obj, res );
But we do have union types now. So this seems doable.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27997#discussion_r2474282523
More information about the hotspot-dev
mailing list