[12] RFR (S): 8215757: C2: PhaseIdealLoop::spinup() computes wrong post-dominating point

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Fri Jan 11 02:01:59 UTC 2019


http://cr.openjdk.java.net/~vlivanov/8215757/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8215757

Crash happens during SplitIf transformation when 
PhaseIdealLoop::spinup() erroneously uses the Region being eliminated as 
a post-dominating merge point (prior_n).

Sequence of events during PhaseIdealLoop pass which leads to the crash 
(IR in question [1]):

   #0: RegionNode 1722 (R1722) starts with IDOM(R1722) = IfNode 1511 (I1511)


   #1: Loop strip mining takes place and inserts new loop limit check:

     Loop: N1572/N1601  limit_check sfpts={ 1595 }
     ...
     Counted    Loop: N1866/N1601  counted [2,int),+1 (-1 iters)
     ...
     Loop: N1865/N1864  limit_check
       Loop: N1866/N1601  limit_check counted [2,int),+1 (-1 iters) 
has_sfpt strip_mined


   #2: As part of loop limit check insertion, new IfNode is created (If 
1854) and linked to R1722 as an input which causes R1722 IDOM to be 
updated [2]. It changes R1722 IDOM (I1511 => R1784), since dom_lca() 
normalizes the result using find_non_split_ctrl().


   #3: SplitIf is performed on I1511 and Phi 1790 is being processed. It 
has 3 users (197, 198, 199) which are attached to R1710, R1716, and 
R1722 respectively. At this point:

        IDOM(R1710) = I1511
        IDOM(R1716) = I1511
        IDOM(R1722) = R1784 <==


   #4: PhaseIdealLoop::handle_use() works fine for 197 & 198:

         197 =idom=> R1710 =idom=> I796 ( == iff_dom)
         198 =idom=> R1716 =idom=> I796

       but fails on 199 when it tries to process R1784 (being 
eliminated) in nested PhaseIdealLoop::spinup() call:

         199 =idom=> R1722 =idom=> R1784 =idom=> I796


The root cause is that while PhaseIdealLoop::do_split_if() updates IDOM 
for If & its projections, it doesn't do that for the corresponding 
Region (R1784) until the splitting is finished [3].

Proposed fix is to take into account delayed IDOM update (region -> 
region_dom) and explicitly check for old Region in 
PhaseIdealLoop::spinup() treating it as iff_dom.

Testing: failing test (replay), hs-precheckin-comp, hs-tier1, hs-tier2 
(in progress)

Thanks!

Best regards,
Vladimir Ivanov

[1] http://cr.openjdk.java.net/~vlivanov/8215757/split_if_1511.png

[2] 
http://hg.openjdk.java.net/jdk/jdk/file/2e1fd6414c4b/src/hotspot/share/opto/loopPredicate.cpp#l161

[3] 
http://hg.openjdk.java.net/jdk/jdk/file/2e1fd6414c4b/src/hotspot/share/opto/split_if.cpp#l525

   void PhaseIdealLoop::do_split_if( Node *iff ) {
     ...
     // Lazy replace IDOM info with the region's dominator
     lazy_replace( iff, region_dom );
     ...
     // Now make the original merge point go dead, by handling all its uses.
     ...
     lazy_replace( region, region_dom );
   }


More information about the hotspot-compiler-dev mailing list