On Mon, 29 Jun 2026 21:32:27 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 2859 commits: >> >> - Merge branch '8317277' into jep401_sub_review_8317278 >> - Merge remote-tracking branch 'valhalla/lworld' into 8317277 >> - Merge >> >> Merge jdk-28+4 >> - 8386963: [lworld] Improve the exception message from Object >> synchronization methods on value objects >> >> Reviewed-by: dholmes, alanb >> - 8387300: [lworld] Minor review comments in javac >> >> Reviewed-by: vromero >> - 8387192: [lworld] Review comment drop for core libs >> >> Reviewed-by: jvernee, vromero >> - 8386999: [lworld] C2: assert(is_dead_loop_safe()) failed: shouldn't be >> cleared yet >> >> Reviewed-by: qamai, vlivanov >> - 8386787: [lworld] >> compiler/valhalla/inlinetypes/TestValueConstruction.java#StressIncrementalInliningDontInlineMyAbstractInit >> timed out >> >> Reviewed-by: phubner, chagedorn >> - 8386995: [lworld] Duplicate value classes are a preview feature warning >> >> Reviewed-by: alanb, vromero >> - 8383389: [lworld] Augment AOTMapLogger::print_oop_details to support flat >> arrays with oops >> >> Reviewed-by: iklam, fparain >> - ... and 2849 more: https://git.openjdk.org/jdk/compare/193de1b1...cffcfb57 > > src/hotspot/share/c1/c1_GraphBuilder.cpp line 2019: > >> 2017: pending_load_indexed()->update(field, offset - >> field->holder()->as_inline_klass()->payload_offset()); >> 2018: } else if (has_pending_field_access()) { >> 2019: pending_field_access()->inc_offset(offset - >> field->holder()->as_inline_klass()->payload_offset()); > > Are we relativizing offsets here? A comment might help. Yes, the offset is being relativized. A flattened nested value contains only its payload, without its object header, so we subtract the holder’s `payload_offset()`. I'll add a comment. > src/hotspot/share/c1/c1_Instruction.cpp line 241: > >> 239: ciType* LoadIndexed::exact_type() const { >> 240: ciType* array_type = array()->exact_type(); >> 241: if (delayed() == nullptr && array_type != nullptr) { > > A comment explaining why delayed() changes the type would be helpful. I'll add + // A delayed load produces a field within the array element. Let + // Instruction::exact_type() below derive its type from declared_type(). > src/hotspot/share/c1/c1_Instruction.cpp line 256: > >> 254: >> 255: ciType* LoadIndexed::declared_type() const { >> 256: if (delayed() != nullptr) { > > A comment explaining why delayed() changes the type would be helpful. > It looked like some kind of instruction fusion, but I'm not seeing much > documentation on it. Right, I'll add + // The LoadIndexed is fused with one or more following getfield bytecodes + // and produces the final field value instead of the array element. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3519648721 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3519653613 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3519657653
