[lworld] RFR: 8335256: [lworld] compiler/valhalla/inlinetypes/TestValueConstruction.java fails intermittently [v2]

[Quan Anh Mai]

On Tue, 29 Apr 2025 08:36:24 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
>> 
>>  - Merge branch 'lworld' into larvaloop
>>  - remove larval InlineTypeNode
>
> Great work @merykitty! Thanks a lot for the investigation. When working on the new value object construction model, I was concerned that we might end up with adding identity back to the larvals but I was still naively hoping that we could get around that. Looks like we can't. Maybe you could change the title to something more descriptive for future reference / search.
> 
> I run this through testing and there are a few issues:
> 
> 
> applications/ctw/modules/jdk_internal_le.java
> -ea -esa -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -server -XX:-TieredCompilation
> 
> # A fatal error has been detected by the Java Runtime Environment:
> #
> #  Internal Error (/workspace/open/src/hotspot/share/opto/macro.cpp:2970), pid=1140043, tid=1140412
> #  assert(!n->as_AbstractLock()->is_eliminated()) failed: sanity
> #
> # JRE version: Java(TM) SE Runtime Environment (25.0) (fastdebug build 25-lworld5ea-LTS-2025-04-28-0856180.tobias.hartmann.valhalla2)
> # Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 25-lworld5ea-LTS-2025-04-28-0856180.tobias.hartmann.valhalla2, mixed mode, sharing, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
> # Problematic frame:
> # V  [libjvm.so+0x15359a4]  PhaseMacroExpand::eliminate_macro_nodes()+0x714
> 
> Current CompileTask:
> C2:26441 3356   !b        jdk.internal.org.jline.utils.NonBlockingInputStreamImpl::run (312 bytes)
> 
> Stack: [0x00007f27dbbfd000,0x00007f27dbcfd000],  sp=0x00007f27dbcf8250,  free space=1004k
> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
> V  [libjvm.so+0x15359a4]  PhaseMacroExpand::eliminate_macro_nodes()+0x714  (macro.cpp:2970)
> V  [libjvm.so+0xb26617]  Compile::Optimize()+0x1867  (compile.cpp:2903)
> V  [libjvm.so+0xb28ceb]  Compile::Compile(ciEnv*, ciMethod*, int, Options, DirectiveSet*)+0x1feb  (compile.cpp:874)
> V  [libjvm.so+0x94e98d]  C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*)+0x46d  (c2compiler.cpp:142)
> V  [libjvm.so+0xb379f2]  CompileBroker::invoke_compiler_on_method(CompileTask*)+0xb22  (compileBroker.cpp:2307)
> V  [libjvm.so+0xb389f8]  CompileBroker::compiler_thread_loop()+0x588  (compileBroker.cpp:1951)
> V  [libjvm.so+0x109191f]  JavaThread::thread_main_inner()+0x12f  (javaThread.cpp:773)
> V  [libjvm.so+0x1abf606]  Thread::call_run()+0xb6  (thread.cpp:231)
> V  [libjvm.so+0x17466d8]  thread_native_entry(Thread*)+0x128  (os_linux.cpp:877)
> 
> 
> 
> 
> compiler/valhalla/inlinetypes/TestValueClasses.java
> -DWarmup=0 -DVerifyIR=false 
> OR
> -Xcomp -XX:-TieredCompilation -D...

@TobiHartmann Thanks a lot for the comprehensive testing, it is really helpful.

> applications/ctw/modules/jdk_internal_le.java
-ea -esa -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -server -XX:-TieredCompilation

This one fails because after removal of some allocations, not only other allocations become eligible for elimination, locks can also be eliminated. As a result, we need to do the whole macro elimination again. I noticed that `_has_lock` is unused, which makes it trivial to merge the 2 loops, simplify the implementation there.

> compiler/valhalla/inlinetypes/TestValueClasses.java
-DWarmup=0 -DVerifyIR=false 
OR
-Xcomp -XX:-TieredCompilation -DIgnoreCompilerControls=true

This one is an oversight by me. When exiting a constructor, the larval oop is aggressively replaced by the non-larval one in all the frames. This relies on the assumption that all blocks in which the oop is larval has been processed. This is incorrect when we have uncommon traps, which can be processed at any time. As a result, we can only replace the larval oop with the non-larval one in the current frame.

> compiler/valhalla/inlinetypes/TestValueConstruction.java
-XX:+UnlockDiagnosticVMOptions -XX:+AbortVMOnCompilationFailure

I modified the test to push everything into the local slots, 2 distinct slots are present into the loop, keep the original intention of the test.

> compiler/valhalla/inlinetypes/TestValueConstruction.java
Various flags:
> - No flags
> - "-XX:+UnlockDiagnosticVMOptions -XX:-TieredCompilation

This is interesting. This is an oversight from #1424. The issue is that it is hard to decide that 2 nodes are definitely different or definitely the same, especially when the graph may be non-canonical during IGVN. As a result, I resort to a more conservative approach there:

- Give up finding a matching constructor for a local allocation.
- When deciding if a call can modify a field of a local allocation, don't take into account the receiver and return `true` for any constructor of a subclass regardless of whether the receiver is actually the object we are loading from.

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

PR Comment: https://git.openjdk.org/valhalla/pull/1447#issuecomment-2840098133