RFR: 8370405: C2: mismatched store from MergeStores wrongly scalarized in allocation elimination [v2]

[Quan Anh Mai]

On Wed, 29 Oct 2025 16:37:08 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Quick summary of investigation:
>> 
>> The `EncodeP` is the `field_val`:
>> 
>> (rr) p field_val->dump_bfs(3,0,"#")
>> dist dump
>> ---------------------------------------------
>>    3  303  CallDynamicJava  === 203 299 202 8 1 (10 40 1 1 90 90 69 ) [[ 304 305 306 308 317 316 ]] # Dynamic  java.lang.ClassLoader::findResources java/lang/Object * ( java/security/SecureClassLoader:NotNull *, java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact * ) ClassLoader::getResources @ bci:42 (line 1445) !jvms: ClassLoader::getResources @ bci:42 (line 1445)
>>    3  332  If  === 311 331  [[ 333 334 ]] P=0.999999, C=-1.000000 !jvms: ClassLoader::getResources @ bci:45 (line 1445)
>>    2  308  Proj  === 303  [[ 335 330 ]] #5  Oop:java/lang/Object * !jvms: ClassLoader::getResources @ bci:42 (line 1445)
>>    2  334  IfTrue  === 332  [[ 376 335 ]] #1 !jvms: ClassLoader::getResources @ bci:45 (line 1445)
>>    1  335  CastPP  === 334 308  [[ 358 343 353 ]]  #java/lang/Object:NotNull *  Oop:java/lang/Object:NotNull * !jvms: ClassLoader::getResources @ bci:45 (line 1445)
>>    0  358  EncodeP  === _ 335  [[ 359 ]]  #narrowoop: java/lang/Object:NotNull * !jvms: ClassLoader::getResources @ bci:45 (line 1445)
>> 
>> It seems the value comes from the call, via a projection, and a null-check cast. But I see no type/interface check cast for `Enumeration`.
>> 
>> This is the store that is being eliminated:
>> 
>> 359  StoreN  === 377 393 322 358  [[ 365 ]]  @narrowoop: java/lang/Object *[int:>=0] (java/lang/Cloneable,java/io/Serializable)+any * [narrow], idx=6;  Memory: @narrowoop: java/lang/Object (java/util/Enumeration) *[int:2] (java/lang/Cloneable,java/io/Serializable):NotNull:exact[1] *,iid=73 [narrow], idx=12; !jvms: ClassLoader::getResources @ bci:45 (line 1445)
>> 
>> 
>> Tracing back where the store comes from: `Parse::array_store`.
>> 
>> (rr) p elemtype->dump()
>> narrowoop: java/lang/Object (java/util/Enumeration) *
>> 
>> Inside `Parse::array_store_check` -> `GraphKit::gen_checkcast` we create a null check, with the CastPP we saw above:
>> 
>> 335  CastPP  === 334 308  [[ 24 ]]  #java/lang/Object:NotNull *  Oop:java/lang/Object:NotNull * !jvms: ClassLoader::getResources @ bci:45 (line 1445)
>> 
>> 
>> More in next comment...
>
> 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.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27997#discussion_r2474273004