On Wed, 22 Apr 2026 12:04:02 GMT, Tobias Hartmann <[email protected]> wrote:
> Addressing various TODOs / FIXMEs in JIT code that don't have a bug number. I > added comments explaining 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). src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp line 702: > 700: case T_ARRAY: > 701: // Non-null case is not handled on aarch64 but handled on x86 > 702: // FIXME: do we need to add it here? This is not Valhalla specific and obviously this path is never reached. Removing. src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp line 1557: > 1555: // (1) Null check -- if one of the operands is null, the other must > not be null (because > 1556: // the two references are not equal), so they are not > substitutable, > 1557: // FIXME: do null check only if the operand is nullable After the removal of Q-types, we don't propagate null-free information for locals through C1 anymore. This is obsolete. src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 690: > 688: // We get an array of objects from the runtime call > 689: __ get_vm_result_oop(buf_array, rthread); > 690: __ get_vm_result_metadata(rmethod, rthread); // TODO: required to > keep the callee Method live? I don't think these (more below) were ever needed - or at least I don't understand why they would be needed. Removing for now. All testing passed. src/hotspot/share/c1/c1_Instruction.cpp line 292: > 290: ciType* NewObjectArray::exact_type() const { > 291: // TODO 8350865 The refined type should be used here > 292: // return ciArrayKlass::make(klass(), false, true, true); This is actually a major limitation. All the fast paths for flat arrays guarded by `Instruction::is_loaded_flat_array()` are not taken anymore. They were also not yet adapted to support nullable / atomic. I'll address this as part of [JDK-8350865](https://bugs.openjdk.org/browse/JDK-8350865) or a subtask. src/hotspot/share/c1/c1_LIR.cpp line 1595: > 1593: void LIR_List::store_check(LIR_Opr object, LIR_Opr array, LIR_Opr tmp1, > LIR_Opr tmp2, LIR_Opr tmp3, > 1594: CodeEmitInfo* info_for_exception, ciMethod* > profiled_method, int profiled_bci) { > 1595: // FIXME -- if the types of the array and/or the object are known > statically, we can avoid loading the klass Not Valhalla specific and not something that I think is beneficial doing in C1. src/hotspot/share/c1/c1_LIRAssembler.cpp line 590: > 588: DebugToken* locvals = debug_info->create_scope_values(nullptr); // > FIXME is this needed (for Java debugging to work properly??) > 589: DebugToken* expvals = debug_info->create_scope_values(nullptr); // > FIXME is this needed (for Java debugging to work properly??) > 590: DebugToken* monvals = debug_info->create_monitor_values(nullptr); // > FIXME: need testing with synchronized method These are not needed. I added a comment explaining why. src/hotspot/share/c1/c1_LIRGenerator.cpp line 1694: > 1692: } > 1693: > 1694: // TODO 8350865 Can we find another way to pass an address to > access_load_at()? I gave this a quick try. It's non-trivial and I couldn't come up with a solution that's better than what we currently have. Will re-visit with JDK-8350865. src/hotspot/share/runtime/deoptimization.cpp line 317: > 315: k = ObjArrayKlass::cast(k)->klass_with_properties(props, THREAD); > 316: } else { > 317: // TODO 8382708 JVMCI does not yet pass properties. Just go with > the default for now. Tracked by [JDK-8382708](https://bugs.openjdk.org/browse/JDK-8382708). src/hotspot/share/runtime/signature_cc.hpp line 78: > 76: _sig_idx = sig_idx; > 77: _regs_idx = regs_idx; > 78: assert(_sig->at(_sig_idx)._bt == ((_step > 0) ? T_METADATA : T_VOID), > "should be at inline type delimiter"); Fix for [JDK-8382641](https://bugs.openjdk.org/browse/JDK-8382641) ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2352#discussion_r3123800109 PR Review Comment: https://git.openjdk.org/valhalla/pull/2352#discussion_r3123803536 PR Review Comment: https://git.openjdk.org/valhalla/pull/2352#discussion_r3123807663 PR Review Comment: https://git.openjdk.org/valhalla/pull/2352#discussion_r3123820606 PR Review Comment: https://git.openjdk.org/valhalla/pull/2352#discussion_r3123823705 PR Review Comment: https://git.openjdk.org/valhalla/pull/2352#discussion_r3123826534 PR Review Comment: https://git.openjdk.org/valhalla/pull/2352#discussion_r3123831269 PR Review Comment: https://git.openjdk.org/valhalla/pull/2352#discussion_r3123787915 PR Review Comment: https://git.openjdk.org/valhalla/pull/2352#discussion_r3123781986
