On Tue, 7 Jul 2026 21:30:57 GMT, Dean Long <[email protected]> wrote:

>> David Simms has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 2900 commits:
>> 
>>  - Merge branch '8317277' into jep401_sub_review_8317278
>>  - Merge remote-tracking branch 'valhalla/lworld' into 8317277
>>  - Merge
>>    
>>    Merge jdk-28+5
>>  - 8386602: [lworld] C2: assert(n != top() || r->in(pnum) == top()) failed: 
>> live value must not be garbage
>>    
>>    Reviewed-by: thartmann
>>  - 8386499: [lworld] java/util/Arrays/ArraysEqCmpTest.java fails with 
>> unexpected layoutKind 5
>>    
>>    Reviewed-by: chagedorn
>>  - 8387629: [lworld] Don't use apiNote for some recent JDI spec updates
>>    
>>    Reviewed-by: sspitsyn
>>  - 8387682: [lworld] Mention preview API in description of 
>> can_support_value_objects capability
>>    
>>    Reviewed-by: cjplummer
>>  - 8387657: [lworld] Object Tags section of JVMTI spec should make it clear 
>> that ObjectFree event is not sent for value objects
>>    
>>    Reviewed-by: sspitsyn, cjplummer
>>  - 8382226: [lworld] C2: Fix _copyOf/_copyOfRange intrinsic for flat 
>> abstract value class arrays
>>    
>>    Reviewed-by: qamai, thartmann
>>  - 8387612: [lworld] C2: assert(array_type->is_flat() || 
>> ideal.ctrl()->in(0)->as_If()->is_flat_array_check(&_gvn)) failed: Should be 
>> found
>>    
>>    Co-authored-by: Marc Chevalier <[email protected]>
>>    Reviewed-by: thartmann
>>  - ... and 2890 more: https://git.openjdk.org/jdk/compare/db987b1e...f1f6d35d
>
> src/hotspot/share/c1/c1_LIRGenerator.cpp line 663:
> 
>> 661:   if (allow_inline) {
>> 662:     assert(!is_unresolved && klass->is_loaded(), "inline type klass 
>> should be resolved");
>> 663:     __ metadata2reg(klass->constant_encoding(), klass_reg);
> 
> This special path seems to mostly bypass PatchALot logic in 
> klass2reg_with_patching.  I'm guessing it isn't necessary and we can just 
> revert to mainline code that always calls klass2reg_with_patching.

When reverting this to mainline and running `TestLWorld.java` with 
`-XX:+PatchALot`, we hit:
https://github.com/openjdk/valhalla/blob/23a199ffb1c27605257f1be6d8a5b8bcfa85e25d/src/hotspot/share/c1/c1_LIRAssembler.cpp#L72

This special path is still required for resolved value class allocations. When 
compiling a flat field access, C1 may generate a `NewInstance` buffer. The 
corresponding `CodeEmitInfo` refers to the `_getfield` or `_putfield` bytecode. 
When we directly go through `klass2reg_with_patching()` here, we attempt to 
attach a `load_klass` patch to such a field access bytecode which is 
unexpected. We only allow these bytecodes:
https://github.com/openjdk/jdk/blob/31dede3f96d78dd0b1c93d84adb1f97b34ff339f/src/hotspot/share/c1/c1_LIRAssembler.cpp#L60-L66

The problem is also described in 
[JDK-8328010](https://bugs.openjdk.org/browse/JDK-8328010).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3550389664

Reply via email to