RFR(S): 8214862: assert(proj != __null) at compile.cpp:3251

[Vladimir Kozlov]

Added bug id to Subject.

Why relaxing assert is not enough? Why you want to delay edge removal?

I would suggest to create a small function (in same file) to call from Compile::Optimize() - we 
don't do graph transformations in it but call other functions.

You don't need to use 'C->' because it is already Compile's method.

Thanks,
Vladimir

On 12/6/18 1:42 AM, Roland Westrelin wrote:
> 
> http://cr.openjdk.java.net/~roland/8214862/webrev.00/
> 
> The crash occurs in final_graph_reshape in the following code:
> 
>    case Op_Proj: {
>      if (OptimizeStringConcat) {
>        ProjNode* p = n->as_Proj();
>        if (p->_is_io_use) {
>          // Separate projections were used for the exception path which
>          // are normally removed by a late inline.  If it wasn't inlined
>          // then they will hang around and should just be replaced with
>          // the original one.
>          Node* proj = NULL;
>          // Replace with just one
>          for (SimpleDUIterator i(p->in(0)); i.has_next(); i.next()) {
>            Node *use = i.get();
>            if (use->is_Proj() && p != use && use->as_Proj()->_con == p->_con) {
>              proj = use;
>              break;
>            }
>          }
>          assert(proj != NULL, "must be found");
>          p->subsume_by(proj, this);
>        }
>      }
>      break;
>    }
> 
> because a call only has control/memory/io uses for the exception path
> but none for the fall through path. This happens because after the call,
> the fallthrough path leads to an infinite loop and that infinite loop
> and the path that leads to it is removed. The infinite loop is removed
> because there are 2 passes of PhaseRemoveUseless that are executed. One
> as part of Compile::inline_string_calls(), and another one right after,
> both at the end of parsing:
> 
>      if (_late_inlines.length() == 0 && !has_mh_late_inlines() && !failing() && has_stringbuilder()) {
>        inline_string_calls(true);
>      }
> 
>      if (failing())  return;
> 
>      print_method(PHASE_BEFORE_REMOVEUSELESS, 3);
> 
>      // Remove clutter produced by parsing.
>      if (!failing()) {
>        ResourceMark rm;
>        PhaseRemoveUseless pru(initial_gvn(), &for_igvn);
>      }
> 
> During parsing, safepoint nodes in infinite loops are kept live with an
> extra precedence edge from the Root node to the safepoint. That edge is
> removed as part of PhaseRemoveUseless. So the first pass of
> PhaseRemoveUseless disconnects the safepoint node in the infinite loop,
> the second finds the safepoint node has no uses and removes it.
> 
> To fix that, I moved the logic that disconnects safepoints from the Root
> node after parsing is over for good, that is after all late inlining
> passes. Optimizations that are applied between the initial parsing phase
> and the removal of the precedence edges, can get confused by the
> precedence edge when a safepoint node goes dead. So I also had to add
> explicit tests for a safepoint node in a few locations.
> 
> That fix is not sufficient to not hit the assert in final graph
> reshape because a safepoint in an infinite loop doesn't have any io
> input. So I also had to relax the code in final graph reshape for io
> projections.
> 
> Roland.
>