On Fri, 24 Apr 2026 06:34:31 GMT, Christian Hagedorn <[email protected]> wrote:
> I did a pass over the compiler code and cleaned up some code. This including: > - Unused variables > - Unused includes > - Make methods `const/static` > - Adding `override` > - Fix code style > - Rename variables that hide outer scope variables > - Remove unused code > - Remove line breaks not present in mainline > > More details in PR comments. > > I did some sanity testing up to tier4 + stress. > > Thanks, > Christian > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). src/hotspot/share/ci/ciInlineKlass.hpp line 80: > 78: address unpack_handler() const; > 79: InlineKlass* get_InlineKlass() const; > 80: int nullable_size_in_bytes() const; Unused and no longer had an implementation src/hotspot/share/ci/ciMethod.hpp line 274: > 272: bool parameter_profiled_type(int i, ciKlass*& type, > ProfilePtrKind& ptr_kind); > 273: bool return_profiled_type(int bci, ciKlass*& type, > ProfilePtrKind& ptr_kind); > 274: bool array_access_profiled_type(int bci, ciKlass*& > array_type, ciKlass*& element_type, ProfilePtrKind& element_ptr, Split long line + fix `&` position. src/hotspot/share/opto/callGenerator.cpp line 188: > 186: if (kit.stopped()) { > 187: return kit.transfer_exceptions_into_jvms(); > 188: } This and the same below were added with [this commit](https://github.com/openjdk/valhalla/commit/9de8ea4d7d75d36d21707e26c062a4cfaf5fc17a#diff-be12257b7cfa0ec96698a5c3958f5aa98723582f68cf2648b376bbe40576b52d) when inserting an uncommon trap to `set_arguments_for_java_call()`: https://github.com/openjdk/valhalla/blob/9de8ea4d7d75d36d21707e26c062a4cfaf5fc17a/src/hotspot/share/opto/graphKit.cpp#L1806 The trap was later removed with JDK-8212190 again which makes the `stopped()` call unneeded: https://github.com/openjdk/valhalla/commit/0a07ea9ba9a6ae110e4874c8bd75f144a5b0a647#diff-be12257b7cfa0ec96698a5c3958f5aa98723582f68cf2648b376bbe40576b52d I removed both checks. src/hotspot/share/opto/convertnode.cpp line 69: > 67: } > 68: > 69: > //------------------------------Ideal------------------------------------------ Is not present in mainline. src/hotspot/share/opto/graphKit.cpp line 1129: > 1127: > 1128: // Update the two tail pointers in parallel. > 1129: callee_jvms = out_jvms; Assigned but never used. src/hotspot/share/opto/graphKit.cpp line 2183: > 2181: for (uint i = TypeFunc::Parms+1; i < domain->cnt(); i++) { > 2182: // Will be rewired later in replace_call(). > 2183: _gvn.transform(new ProjNode(call, i)); Was recently added with JDK-8369045 by @dafedafe: https://github.com/openjdk/valhalla/commit/4ffaf1477da5e8236cf13a45bff0a760bcf0cea9#diff-be12257b7cfa0ec96698a5c3958f5aa98723582f68cf2648b376bbe40576b52d Double-checked with him: We will later rewire the projections in `replace_call()`. Added a comment and removed the unused local `proj`. src/hotspot/share/opto/inlinetypenode.cpp line 714: > 712: if (this_field->is_InlineType()) { > 713: RegionNode* done_region = new RegionNode(1); > 714: ciField* field = this->field(i); This hides the outer-scope `field` variable which is also set to the exact same value: https://github.com/openjdk/valhalla/blob/f09dea673b68e51ff3604d7c6f0f0561dd0f8c98/src/hotspot/share/opto/inlinetypenode.cpp#L679 Removed this unneeded identical assignment. src/hotspot/share/opto/inlinetypenode.cpp line 1044: > 1042: int old_len = visited.length(); > 1043: visited.push(ft); > 1044: ciInlineKlass* vk_field = ft->as_inline_klass(); Hides outer-scope `vk` variable. Renamed. src/hotspot/share/opto/inlinetypenode.cpp line 1465: > 1463: if (!field->is_null_free()) { > 1464: assert(field->null_marker_offset() != -1, "inconsistency"); > 1465: Node* null_marker = nullptr; Hides outer-scope `null_marker` variable. Renamed. src/hotspot/share/opto/inlinetypenode.hpp line 184: > 182: DecoratorSet _decorators; > 183: > 184: uint size_of() const override { return sizeof(*this); } Moved up here and simplified explicit name with `*this` which we often do at other places. Same below for `LoadFlatNode`. src/hotspot/share/opto/inlinetypenode.hpp line 198: > 196: Node* Ideal(PhaseGVN* phase, bool can_reshape) override { return > nullptr; } > 197: Node* Identity(PhaseGVN* phase) override { return this; } > 198: const Type* Value(PhaseGVN* phase) const override; These were private which is unexpected when the base methods are already public. Moved them to the public block. Same below for `LoadFlatNode`. src/hotspot/share/opto/type.cpp line 3983: > 3981: flat = array_klass->is_flat_array_klass(); > 3982: not_flat = !flat; > 3983: bool is_null_free = array_klass->is_elem_null_free(); Only used inside this if-scope. src/hotspot/share/opto/type.cpp line 7112: > 7110: interfaces = this_interfaces->intersection_with(tp_interfaces); > 7111: FlatInArray flat_in_array = meet_flat_in_array(NotFlat, > tp->flat_in_array()); > 7112: return TypeInstKlassPtr::make(ptr, > ciEnv::current()->Object_klass(), interfaces, offset, flat_in_array); `flat_in_array` is unused. This looks like a real issue from JDK-8332406: https://github.com/openjdk/valhalla/commit/a826fbd06ab41c483f59c06fed1757fae8c3a183#diff-3559dcf23b719805be5fd06fd5c1851dbd8f53e47afe6d99cba13a3de0ebc6b2 We compute the "flat in array meet" but do not propagate it to the newly created type. This looks like a mistake but never resulted in an observable failure. Not sure if it's worth a separate issue and thus included here. Testing looked good. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135885373 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135887792 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135918168 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135921833 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135922620 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135933629 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135943147 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135944627 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135947409 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135952949 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135955582 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135957700 PR Review Comment: https://git.openjdk.org/valhalla/pull/2364#discussion_r3135972428
