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

[Vladimir Kozlov]

I am fine with this conservative fix (bailout optimization). It is good that performance is not affected.

Thanks,
Vladimir

On 11/5/19 1:01 AM, Tobias Hartmann wrote:
> Performance results look good.
> 
> Best regards,
> Tobias
> 
> On 04.11.19 08:25, Tobias Hartmann wrote:
>> 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.
>>>