RFR(S): 8232539: SIGSEGV in C2 Node::unique_ctrl_out

[Tobias Hartmann]

Hi Roland,

this seems reasonable to me but I'm concerned that it might cause performance regressions. I'll run
some tests in our system.

Best regards,
Tobias

On 23.10.19 10:50, Roland Westrelin wrote:
> 
> http://cr.openjdk.java.net/~roland/8232539/webrev.00/
> 
> I couldn't come up with a test case because node processing order during
> IGVN matters. Bug was reported against 11 but I see no reason it
> wouldn't apply to current code as well.
> 
> At parse time, predicates are added by
> Parse::maybe_add_predicate_after_if() but not loop is actually
> created. Compile::_major_progress is cleared. On the next round of IGVN,
> one input of a region points to predicates. The same region has an if as
> use that can be split through phi during IGVN. The predicates are going
> to be removed by IGVN. But that happens in multiple steps because there
> are several predicates (for reason Deoptimization::Reason_predicate,
> Deoptimization::Reason_loop_limit_check etc.) and because for each
> predicate one IGVN iteration must first remove the Opaque1 node, then
> another kill the IfFalse projection, finally another replace the IfTrue
> projection by the If control input.
> 
> Split if occurs while predicates are in the process of being removed. It
> sees predicates, tries to walk over them, encounters a predicates that's
> been half removed (false projection removed) and we hit the assert/crash.
> 
> I propose we simply not apply IGVN split if if we're splitting through a
> loop or if there's a predicate input to a region because:
> 
> - Making split if robust to dying predicates is not straightforward as
>   far as I can tell
> 
> - Loop opts split if doesn't split through loop header so why would it
>   make sense for IGVN split if?
> 
> - I'm wondering if there are other cases where handling of predicates in
>   split if could be wrong (and so more trouble ahead):
> 
>   + What if we split through a Loop region, predicates were added by
>   loop optimizations, loop opts are now over so the predicates added at
>   parse time were removed: then PhaseIdealLoop::find_predicate()
>   wouldn't report a predicate but cloning predicates would still be
>   required for correctness?
> 
>   + What if we have no loop, a region has predicates as input,
>   predicates are going to die but have not yet been processed, split if
>   uselessly duplicates predicates but one of then is control dependent
>   on the branch it is in so cloning predicates actually causes a broken
>   graph?
> 
> So overall it feels safer to me to simply bail out from split if for
> loops/predicates.
> 
> Roland.
>