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

[Tobias Hartmann]

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.
>>