[lworld] RFR: 8357474: [lworld] Consolidate load/store flat inline types [v3]

[Tobias Hartmann]

On Thu, 22 May 2025 05:56:47 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Hi,
>> 
>> This patch consolidates loading and storing of flat inline types. This makes it so that `make_from_flat` and `store_flat` will take a pointer to the target location. `make_from_flat_array` and `store_flat_array` are split out into their own methods which receive an `idx` parameter. This also fixes some issues I theorycrafted:
>> 
>> - Improper loading/storing of atomic elements in a non-atomic container. This is not supported at the moment but it would be great to prepare for it.
>> - `is_naturally_atomic` checks `nof_declared_nonstatic_fields`, which is incorrect if the sole field has multiple fields. I refactored it into `ciInlineKlass`, the decision to do atomic loads/stores is also delegated to the callee, the caller only needs to know whether the operations act as if they are atomic, not whether they are actually executed atomically.
>> - `null_free` of an oop can only be trusted if the container itself is null-free. For example this case:
>> 
>>         value class StringHolder {
>>             @NullRestricted
>>             String s;
>>         }
>> 
>>         value class StringHolderHolder {
>>             // flatten
>>             StringHolder v;
>>         }
>> 
>> Then `v.s` cannot be trusted to be non-null because `v` can be null.
>> 
>> This also allows straightforward implementation of https://bugs.openjdk.org/browse/JDK-8349110.
>> 
>> Please let me know what you think. Thanks very much.
>
> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   array

This is a nice refactoring, thanks for working on this!

I'll review the code in detail soon but I submitted some testing already which revealed an issue:

compiler/valhalla/inlinetypes/TestArrayNullMarkers.java
-Xcomp -XX:-TieredCompilation -DIgnoreCompilerControls=true

# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/workspace/open/src/hotspot/share/opto/memnode.cpp:3587), pid=2958342, tid=2958358
#  assert(Opcode() == st->Opcode() || st->Opcode() == Op_StoreVector || Opcode() == Op_StoreVector || st->Opcode() == Op_StoreVectorScatter || Opcode() == Op_StoreVectorScatter || phase->C->get_alias_index(adr_type()) == Compile::AliasIdxRaw || (Opcode() == Op_StoreL && st->Opcode() == Op_StoreI) || (Opcode() == Op_StoreI && st->Opcode() == Op_StoreL) || (Opcode() == Op_StoreL && st->Opcode() == Op_StoreN) || (st->adr_type()->isa_aryptr() && st->adr_type()->is_aryptr()->is_flat()) || (is_mismatched_access() || st->as_Store()->is_mismatched_access())) failed: no mismatched stores, except on raw memory: StoreB StoreN
#
# JRE version: Java(TM) SE Runtime Environment (25.0) (fastdebug build 25-lworld5ea-LTS-2025-05-22-0636300.tobias.hartmann.valhalla2)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 25-lworld5ea-LTS-2025-05-22-0636300.tobias.hartmann.valhalla2, mixed mode, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x167db09]  StoreNode::Ideal(PhaseGVN*, bool)+0x2a9

Current CompileTask:
C2:18680  865   !b        compiler.valhalla.inlinetypes.TestArrayNullMarkers::main (2883 bytes)

Stack: [0x00007f99f351b000,0x00007f99f361b000],  sp=0x00007f99f36160b0,  free space=1004k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.so+0x167db09]  StoreNode::Ideal(PhaseGVN*, bool)+0x2a9  (memnode.cpp:3587)
V  [libjvm.so+0x185de4d]  PhaseIterGVN::transform_old(Node*)+0xbd  (phaseX.cpp:668)
V  [libjvm.so+0x18530d4]  PhaseIterGVN::optimize()+0xb4  (phaseX.cpp:1054)
V  [libjvm.so+0xb53b16]  Compile::Optimize()+0x326  (compile.cpp:2779)
V  [libjvm.so+0xb5773f]  Compile::Compile(ciEnv*, ciMethod*, int, Options, DirectiveSet*)+0x1fef  (compile.cpp:874)
V  [libjvm.so+0x96db34]  C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*)+0x484  (c2compiler.cpp:142)
V  [libjvm.so+0xb653f8]  CompileBroker::invoke_compiler_on_method(CompileTask*)+0xb58  (compileBroker.cpp:2307)
V  [libjvm.so+0xb665b8]  CompileBroker::compiler_thread_loop()+0x568  (compileBroker.cpp:1951)
V  [libjvm.so+0x10df77b]  JavaThread::thread_main_inner()+0x13b  (javaThread.cpp:774)
V  [libjvm.so+0x1b595c6]  Thread::call_run()+0xb6  (thread.cpp:231)
V  [libjvm.so+0x17c4af8]  thread_native_entry(Thread*)+0x128  (os_linux.cpp:875)

test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestBasicFunctionality.java line 1154:

> 1152: 
> 1153:     @Test
> 1154:     // TODO more aggressive flattening

We should reference a tracking bug/RFE for this. Same below.

-------------

Changes requested by thartmann (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1470#pullrequestreview-2860939144
PR Review Comment: https://git.openjdk.org/valhalla/pull/1470#discussion_r2102377575