RFR: 8295867: TestVerifyGraphEdges.java fails with exit code -1073741571 when using AlwaysIncrementalInline [v2]

[Christian Hagedorn]

On Thu, 10 Nov 2022 18:30:18 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> When we use -Xcomp we compile `java.lang.invoke.LambdaForm$Kind::<clinit>` very long linear method for enum class: [LambdaForm.java#L250](https://github.com/openjdk/jdk/blame/master/src/java.base/share/classes/java/lang/invoke/LambdaForm.java#L250)
>> 
>> In addition we inline all <init> class initializers for EA when run with -Xcomp: [bytecodeInfo.cpp#L410](https://github.com/openjdk/jdk/blame/master/src/hotspot/share/opto/bytecodeInfo.cpp#L410)
>> 
>> Recent CDS change [JDK-8293979](https://bugs.openjdk.org/browse/JDK-8293979) allows to inline a bit more deeply too.
>> 
>> Adding `-XX:+AlwaysIncrementalInline` worsened situation even more.
>> 
>> Running with `-XX:+LogCompilation` shows that we hit `NodeCountInliningCutoff (18000)` during `java.lang.invoke.LambdaForm$Kind::<clinit>` compilation.
>> 
>> In short, we have very long (>40000 live nodes) linear IR graph. `Node::verify_edges()` method process nodes depth-first starting from first input which is control edge. So it is not surprise that depth of this method recursion reached 6000.
>> With frame size of 10 words (320 bytes) we easy hit stack overflow (768K in 32-bits debug VM).
>> 
>> I fixed it by using local buffer `Node_List` instead of recursion in `Node::verify_edges()`.
>> The algorithm was changed to simplify code. It processes inputs in reverse order - last input processed first. And I noticed that maximum use of buffer is only about 1000 or less elements for this compilation (that is why I use live_nodes/16 as initial size of buffer).
>> 
>> Then I did additional experiment with keeping recursion but processing inputs in reverse order:
>> 
>> 
>>    // Recursive walk over all input edges
>> -  for( i = 0; i < len(); i++ ) {
>> -    n = in(i);
>> +  for( i = len(); i > 0; i++ ) {
>> +    n = in(i - 1);
>>      if( n != NULL )
>>        in(i)->verify_edges(visited);
>>    }
>> 
>> 
>> And it shows the same around 1000 stack depth!
>> 
>> I decided to keep my original fix because it should be faster (put only one value on list instead of putting all locals, PC, SP on stack and calls) and much less stack usage.
>> 
>> Testing tier1-3, hs-comp-stress and `TestVerifyGraphEdges.java` test runs with `-XX:+AlwaysIncrementalInline`.
>
> Vladimir Kozlov 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 five additional commits since the last revision:
> 
>  - Rename verify_edges() method and move it to Compile class
>  - Merge branch 'master' into JDK-8295867
>  - remove white space
>  - Merge branch 'master' into JDK-8295867
>  - 8295867: TestVerifyGraphEdges.java fails with exit code -1073741571 when using AlwaysIncrementalInline

Thanks for doing the updates, looks good!

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

Marked as reviewed by chagedorn (Reviewer).

PR: https://git.openjdk.org/jdk/pull/11065