On Mon, 20 Apr 2026 12:31:27 GMT, Tobias Hartmann <[email protected]> wrote:
> This patch addresses various follow-up TODOs from > [JDK-8278390](https://bugs.openjdk.org/browse/JDK-8278390). > > I added PR comments to explain the changes in more detail. > > Thanks, > Tobias > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). A first pass through the `opto` and `ci` changes from me. src/hotspot/share/ci/ciInlineKlass.hpp line 65: > 63: bool maybe_flat_in_array() const; > 64: bool is_always_flat_in_array() const; > 65: bool can_be_passed_as_fields() const; I think a description of how these values are passed in and out, or a pointer to an existing description is appreciated here, src/hotspot/share/opto/subnode.cpp line 1158: > 1156: return new CmpINode(uncast_in1->as_InlineType()->get_null_marker(), > phase->intcon(0)); > 1157: } > 1158: if (uncast_in1->is_InlineType() || uncast_in2->is_InlineType()) { This looks pretty suspicious tbh. What if we want to fast-path the substitutability check in C2, and compare 2 objects by pointer before looking at each of their fields? src/hotspot/share/opto/type.cpp line 7333: > 7331: > 7332: > //------------------------------make------------------------------------------- > 7333: const TypeFunc* TypeFunc::make(ciMethod* method, bool is_call, bool > is_osr_compilation) { It is better to have a description for the meaning of `is_call`. src/hotspot/share/runtime/sharedRuntime.cpp line 2902: > 2900: if (!_method->is_static()) { > 2901: // We shouldn't scalarize 'this' in a value class constructor > 2902: if (InlineTypePassFieldsAsArgs && holder->is_inline_klass() && > InlineKlass::cast(holder)->can_be_passed_as_fields() && Do you think it is a good idea to reduce the sites in which these flag checks are necessary? In this particular instance, principally, I think `InlineKlass::can_be_passed_as_fields` should return `false` if `InlineTypePassFieldsAsArgs` is `false`, removing the need to explicitly check the flag here. src/hotspot/share/runtime/sharedRuntime.cpp line 2918: > 2916: for (SignatureStream ss(_method->signature()); > !ss.at_return_type(); ss.next()) { > 2917: BasicType bt = ss.type(); > 2918: if (InlineTypePassFieldsAsArgs && bt == T_OBJECT) { Similarly, this can also be removed. ------------- PR Review: https://git.openjdk.org/valhalla/pull/2339#pullrequestreview-4140931659 PR Review Comment: https://git.openjdk.org/valhalla/pull/2339#discussion_r3111525763 PR Review Comment: https://git.openjdk.org/valhalla/pull/2339#discussion_r3111493606 PR Review Comment: https://git.openjdk.org/valhalla/pull/2339#discussion_r3111475146 PR Review Comment: https://git.openjdk.org/valhalla/pull/2339#discussion_r3111455970 PR Review Comment: https://git.openjdk.org/valhalla/pull/2339#discussion_r3111458893
