On Tue, 7 Jul 2026 23:57:25 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 777: > >> 775: if (expected_type != nullptr && >> expected_type->is_obj_array_klass()) { >> 776: // For a direct pointer comparison, we need the refined array >> klass pointer >> 777: expected_type = >> ciObjArrayKlass::make(expected_type->as_array_klass()->element_klass()); > > It would be clearer what was happening here if the "refined_type" argument to > make() wasn't hidden as the default. > Suggestion: > > expected_type = > ciObjArrayKlass::make(expected_type->as_array_klass()->element_klass(), true > /* refined_type */); > > Also, is it possible that `expected_type` is already refined here? It could very well be already refined. We should add a guard for that. Addressing with [JDK-8388007](https://bugs.openjdk.org/browse/JDK-8388007). > src/hotspot/share/c1/c1_LIRGenerator.cpp line 1492: > >> 1490: LIR_Opr LIRGenerator::load_constant(LIR_Const* c) { >> 1491: BasicType t = c->type(); >> 1492: for (int i = 0; i < _constants.length() && !in_conditional_code(); >> i++) { > > It's a little hard to follow what is supposed to happen (and why) when > in_conditional_code() is true here and below. It might be clearer to put all > the in_conditional_code() code at the top along with some comments. That's true, let's make that more readable. Addressing with [JDK-8388007](https://bugs.openjdk.org/browse/JDK-8388007). > src/hotspot/share/c1/c1_Runtime1.cpp line 386: > >> 384: } >> 385: >> 386: static void allocate_instance(JavaThread* current, Klass* klass, TRAPS) >> { > > This function is only used by Runtime1::new_instance below, so there is no > reason to split it out. Can we revert back to jdk mainline here? Looks like it had more callers in the past but not anymore. Let's inline it directly. Addressing with [JDK-8388007](https://bugs.openjdk.org/browse/JDK-8388007). > src/hotspot/share/c1/c1_Runtime1.cpp line 401: > >> 399: oop obj = h->allocate_instance(CHECK); >> 400: current->set_vm_result_oop(obj); >> 401: JRT_END > > JRT_END should be removed it we are keeping this helper function, but I think > it's better to revert these unneeded changes. Addressing with [JDK-8388007](https://bugs.openjdk.org/browse/JDK-8388007) by directly inlining the method. > src/hotspot/share/c1/c1_Runtime1.cpp line 1191: > >> 1189: Klass* ek = >> caller_method->constants()->klass_at(anew.index(), CHECK); >> 1190: k = ek->array_klass(CHECK); >> 1191: if (!k->is_typeArray_klass() && !k->is_refArray_klass() && >> !k->is_flatArray_klass()) { > > Suggestion: > > if (k->is_unrefined_objArray_klass()) { Addressing with [JDK-8388007](https://bugs.openjdk.org/browse/JDK-8388007). > src/hotspot/share/ci/bcEscapeAnalyzer.cpp line 561: > >> 559: // the size of a reference. However, if OFFSET_ANY is given as >> 560: // parameter to set_modified(), size is not taken into account. >> 561: set_modified(arr, OFFSET_ANY, type2size[T_OBJECT]*HeapWordSize); > > It looks a little strange to compute the size when the implementation ignores > it. One could argue that the caller doesn't necessarily know this, but the > comment says otherwise. > We could replace all of these > > set_modified(arr, OFFSET_ANY, type2size[basic_type]*HeapWordSize); > > with something like > > set_modified_any_offset(arr); // implies OFFSET_ANY, or > set_modified(arr); // implies OFFSET_ANY Good idea, addressing with [JDK-8388007](https://bugs.openjdk.org/browse/JDK-8388007). > src/hotspot/share/ci/ciObjArrayKlass.cpp line 193: > >> 191: } >> 192: >> 193: ciArrayKlass* ciObjArrayKlass::make(ciKlass* element_klass, int dims) { > > Suggestion: > > ciObjArrayKlass* ciObjArrayKlass::make(ciKlass* element_klass, int dims) { Addressing with [JDK-8388007](https://bugs.openjdk.org/browse/JDK-8388007). > src/hotspot/share/ci/ciObjArrayKlass.cpp line 198: > >> 196: klass = ciObjArrayKlass::make(klass, /* refined_type = */ false); >> 197: } >> 198: return klass->as_array_klass(); > > Suggestion: > > return klass->as_obj_array_klass(); > > These changes don't look needed anymore. Addressing with [JDK-8388007](https://bugs.openjdk.org/browse/JDK-8388007). > src/hotspot/share/ci/ciObjArrayKlass.hpp line 73: > >> 71: >> 72: static ciObjArrayKlass* make(ciKlass* element_klass, bool refined_type >> = true, bool null_free = false, bool atomic = true); >> 73: static ciArrayKlass* make(ciKlass* element_klass, int dims); > > Suggestion: > > static ciObjArrayKlass* make(ciKlass* element_klass, int dims); Addressing with [JDK-8388007](https://bugs.openjdk.org/browse/JDK-8388007). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3551953653 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3551957890 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3551961143 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3551965265 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3551972077 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3551952539 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3551955095 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3551954317 PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3551955487
