RFR(S): 8240576: JVM crashes after transformation in C2 IdealLoopTree::merge_many_backedges

[Vladimir Kozlov]

Looks reasonable.

Thanks,
Vladimir

On 3/5/20 12:51 AM, Yangfei (Felix) wrote:
> Hi,
> 
>    Please help review this patch fixing a C2 bug.
>    Bug: https://bugs.openjdk.java.net/browse/JDK-8240576
>    Will prepare a webrev adding the test case later.
> 
>    There is an ordinary nested loop in the test case.
>    When entrybci = -1. It has 5 reducible loop.
>    Normal control flow looks like:
> 
>            +------+
>            |0:Root |
>            +------+
>              |
>              |
>              v
>         +---------+
>   <-----  |1:Region1|<-----------------+
>         |if(i<100)|<---------+     |
>         +---------+       |     |
>            |  ^       |     |
>            |  |       |      |
>            v  |       |     |
>        +-----------+      |      |
>        |2:if(mF!=0)|    |      |
>        +-----------+      |      |
>             |         |     |
>             |         |     |
>             v         |     |
>         +---------+       |     | try-catch
>         |3:Region2|    |      |
>         |if(j<101)|----------+      |
>         +---------+              |
>            |  ^              |
>            |  |              |
>            v  |              |
>         +---------+              |
>         |4:Region3|           |
>         |if(k<102)|             |
>         +---------+               |
>            |  ^               |
>            |  |                |
>            v  |                |
> +-----------------------------+           |
> |5:int mI=new Interger(0x1234)|-----+
> +-----------------------------+
> 
> Current CompileTask:
> C2:  13694  832 % !b        Test::testMethod @ 37 (83 bytes)
> 
> Problem triggered during the OSR compilation.
> It has 3 reducible loop and 1 irreducible loop.
> OSR control flow looks like:
> 
>                                             +------+
>                                             |0:Root|
>                                             +---+--+
>                                                |
>                                                |
>                                                v
> +-----------------------------+                       +----+----+
> |5:int mI=new Interger(0x1234)|    <---------    |4:Region3|
> |                             |--------->    |if(k<102)|
> +------------+----------------+                      +--+--+---+
>               |                              |  ^
>               |                              |  |
>               |                              v  |
>               |                           +---------+
>               |                           |3:Region2|<-----------+
>               |  try-catch                  |if(j<101)|         |
>               |                           +---------+          |
>               |                                |           |
>               |                                |           |
>               |                                v           |
>               |                           +---------+          |
>               +-------------------------->           |1:Region1|------>    |
>                                           |if(i<100)|         |
>                                           +---------+          |
>                                              |  ^          |
>                                              |  |          |
>                                              v  |          |
>                                          +-----------+         |
>                                          |2:if(mF!=0)|-----------+
>                                          +-----------+
> 
> In the first beautify_loops, something went wrong when c2 does merge_many_backedges.
> 
> 2090   // If I am a shared header (multiple backedges), peel off the many
> 2091   // backedges into a private merge point and use the merge point as
> 2092   // the one true backedge.
> 2093   if( _head->req() > 3) {
> 2094     // Merge the many backedges into a single backedge but leave
> 2095     // the hottest backedge as separate edge for the following peel.
> 2096     merge_many_backedges( phase );
> 2097     result = true;
> 2098   }
> 
> Subgraph before merge_many_backedges:
> 
> |                              |
> |                              |
> |                              v
> |                           +---------+
> |  try-catch                 |3:Region2|<-----------+
> |                           |if(j<101)|        |
> |                           +---------+          |
> |                                |           |
> |                                |           |
> |                                v            |
> |                            +---------+         |
> +-------------------------->           |1:Region1|  ----->  |
>                              |if(i<100)|         |
>                              +---------+          |
>                                 |  ^          |
>                                 |  |          |
>                                 v  |          |
>                              +-----------+         |
>                              |2:if(mF!=0)|-----------+
>                              +-----------+
> 
> 
> Before line 2096, IdealLoopTree looks like:
> 
> IdealLoopTree:
> N1/N3 irreducible
>    N1/N2 reducible
> 
> 
> Subgraph after merge_many_backedges:
> 
> |                              |
> |                              |
> |                              v
> |                           +--+------+
> |  try+catch                 |3:Region2|<-----------+
> |                           |if(j<101)|         |
> |                           +---------+           |
> |                                |             |
> |                                |             |
> |                                v             |
> |                            +----+----+          |
> +-------------------------->           |1:Region1|----->     |
>                              |if(i<100)|           |
>                               ++-------++          |
>                               |       ^         |
>                               |       |         |
>                               |       |         |
>                               |     +---------+     |
>                               |    |6:Region3|   |
>                               |     +---------+     |
>                               |       ^         |
>                               |       |         |
>                               v       |         |
>                               +-----------+         |
>                               |2:if(mF!=0)| -----------+
>                               +-----------+
> 
> After line 2096, IdealLoopTree looks like:
> 
> IdealLoopTree:
> N1/N6 irreducible
>     N1/N3 reducible
> 
> The overall structure of the loop remains the same, but its irreducible attributes
> has been modified incorrectly, which further leads to the vm crash.
> Proposed fix prevents doing merge_many_backedges when IdealLoopTree is irreducible.
> 
> diff -r 67cc6f3948e3 src/hotspot/share/opto/loopnode.cpp
> --- a/src/hotspot/share/opto/loopnode.cpp       Wed Mar 04 15:34:53 2020 -0800
> +++ b/src/hotspot/share/opto/loopnode.cpp       Thu Mar 05 09:32:57 2020 +0800
> @@ -2091,7 +2091,7 @@
>     // If I am a shared header (multiple backedges), peel off the many
>     // backedges into a private merge point and use the merge point as
>     // the one true backedge.
> -  if( _head->req() > 3 ) {
> +  if (_head->req() > 3 && !_irreducible) {
>       // Merge the many backedges into a single backedge but leave
>       // the hottest backedge as separate edge for the following peel.
>       merge_many_backedges( phase );
> 
>    Tiered 1-3 tested.  Any comment?
> 
> Thanks,
> Felix
>