On Tue, 30 Jun 2026 05:10:41 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_Instruction.cpp line 275: > >> 273: // The following check can fail with inlining: >> 274: // void test45_inline(Object[] oa, Object o, int index) { >> oa[index] = o; } >> 275: // void test45(MyValue1[] va, int index, MyValue2 v) { >> test45_inline(va, v, index); } > > What is this comment trying to say, that it is OK to return false if we lack > exact information? > Saying it can "fail" makes it sound like a bug that needs fixing. > Also, I would expect the opposite: that the check works with inlining, > because we see the true types from the caller and not the erased types from > the callee. Right, Inlining exposes the more specific caller types: `element_klass` is `MyValue1`, while `actual_klass` is `MyValue2`. Their mismatch means the array store check must be retained. I'll adjust the comment. > src/hotspot/share/c1/c1_Instruction.cpp line 293: > >> 291: >> 292: ciType* NewObjectArray::exact_type() const { >> 293: // Returns the refined type > > Is "refined type" defined somewhere? Is it different from "exact type"? As > is, this comment seems to bring up more questions than answers. It's a concept defined by the runtime, see this: https://github.com/openjdk/valhalla/blob/2af7c2350b22ed03850b6b6c146b622ca758f441/src/hotspot/share/oops/klass.hpp#L727 I'll adjust the comment. “Refined” is too implementation-specific here. The point is that anewarray’s default array properties are resolved to the concrete layout klass (reference or flat) which is the exact type of the allocation. @fparain Do we have a comment that explains what "refined" means somewhere? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3519723828 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3519695012
