Safepoint timeout in scimark?

[Vladimir Kozlov]

Thank you, Roland, for analysis.

I am worry that moving safepoint before CLE will be correct in all cases 
since JVMS could be different.

We can do several thins:
1) prevent such case to happen - why SP not before If node?
2) don't convert loop to counted when UseCountedLoopSafepoints in SP on 
back edge.
3) remove such SP regardless UseCountedLoopSafepoints value.

Thanks,
Vladimir

On 7/19/16 8:37 AM, Roland Westrelin wrote:
>> BTW, I tried -XX:+UseCountedLoopSafepoints, and it does indeed solve
>> the problem, but ran into other problems in SPECjvm2008's compiler
>> benchmarks:
>>
>> java -XX:+UseCountedLoopSafepoints -XX:+SafepointTimeout
>> -XX:SafepointTimeoutDelay=2000 -jar SPECjvm2008.jar -ict -ikv compiler
>>
>> .. fails with:
>>
>> #
>> # A fatal error has been detected by the Java Runtime Environment:
>> #
>> #  SIGSEGV (0xb) at pc=0x00007f7adff09814, pid=23268, tid=23296
>> #
>> # JRE version: OpenJDK Runtime Environment (9.0) (fastdebug build 9-
>> internal+0-2016-07-11-184003.rkennke.jdk9)
>> # Java VM: OpenJDK 64-Bit Server VM (fastdebug 9-internal+0-2016-07-11-
>> 184003.rkennke.jdk9, mixed mode, tiered, compressed oops, g1 gc, linux-
>> amd64)
>> # Problematic frame:
>> # V  [libjvm.so+0xfb0814]  PhaseIdealLoop::get_ctrl(Node*)+0x64
>
> I took a look at the crash that Roman observe and I see that the failure
> occurs when a loop has a safepoint on the backedge. In that case,
> because in PhaseIdealLoop::is_counted_loop():
>
>   // Allow funny placement of Safepoint
>   if (back_control->Opcode() == Op_SafePoint)
>     back_control = back_control->in(TypeFunc::Control);
>
> the safepoint node is disconnected when the counted loop is created but
> it's still in the list of safepts so IdealLoopTree::remove_safepoints()
> removes it (because the loop has a call):
>
> phase->lazy_replace(n, n->in(TypeFunc::Control));
>
> with n->in(TypeFunc::Control) = NULL. The loop's tail is left to point
> to the safepoint and when IdealLoopTree::tail() is called later on,
> get_ctrl() is called with NULL. That problem doesn't occur when
> safepoints are optimized out because. I suppose it's because:
>
>   if (!UseCountedLoopSafepoints) {
>     // Check for SafePoint on backedge and remove
>     Node *sfpt = x->in(LoopNode::LoopBackControl);
>     if (sfpt->Opcode() == Op_SafePoint && is_deleteable_safept(sfpt)) {
>       lazy_replace( sfpt, iftrue );
>       if (loop->_safepts != NULL) {
>         loop->_safepts->yank(sfpt);
>       }
>       loop->_tail = iftrue;
>     }
>   }
>
> in PhaseIdealLoop::is_counted_loop()
>
> Anyway, I see 2 problems here: 1) the crash and 2) we drop a safepoint
> that we would like to keep. I tried a fix with the patch below: I moved
> the safepoint above the CountedLoopEnd. I duplicated the test of the
> CountedLoopEnd and put the safepoint in one of the branch otherwise if
> we deoptimize at the safepoint we would resume execution after a test
> that's never executed.
>
> Roland.
>
>
> diff --git a/src/share/vm/opto/loopnode.cpp b/src/share/vm/opto/loopnode.cpp
> --- a/src/share/vm/opto/loopnode.cpp
> +++ b/src/share/vm/opto/loopnode.cpp
> @@ -640,6 +640,32 @@
>    // Replace the old IfNode with a new LoopEndNode
>    Node *lex = _igvn.register_new_node_with_optimizer(new CountedLoopEndNode( iff->in(0), test, cl_prob, iff->as_If()->_fcnt ));
>    IfNode *le = lex->as_If();
> +
> +  Node *sfpt = x->in(LoopNode::LoopBackControl);
> +  if (sfpt->Opcode() == Op_SafePoint && UseCountedLoopSafepoints) {
> +    Node* ctrl = le->in(0);
> +    IfNode* iff = new IfNode(ctrl, test, cl_prob, le->_fcnt);
> +    _igvn.register_new_node_with_optimizer(iff);
> +    set_idom(iff, ctrl, dom_depth(ctrl)+1);
> +    set_loop(iff, loop);
> +    Node* iffalse = _igvn.register_new_node_with_optimizer(new IfFalseNode(iff));
> +    Node* iftrue = _igvn.register_new_node_with_optimizer(new IfTrueNode(iff));
> +    set_idom(iffalse, iff, dom_depth(iff)+1);
> +    set_loop(iffalse, loop);
> +    set_idom(iftrue, iff, dom_depth(iff)+1);
> +    set_loop(iftrue, loop);
> +    _igvn.replace_input_of(sfpt, 0, iftrue);
> +    set_idom(sfpt, iftrue, dom_depth(iftrue)+1);
> +    Node* region = new RegionNode(3);
> +    region->init_req(1, sfpt);
> +    region->init_req(2, iffalse);
> +    _igvn.register_new_node_with_optimizer(region);
> +    set_idom(region, iff, dom_depth(iff)+1);
> +    set_loop(region, loop);
> +    _igvn.replace_input_of(le, 0, region);
> +    loop->_tail = iftrue;
> +  }
> +
>    uint dd = dom_depth(iff);
>    set_idom(le, le->in(0), dd); // Update dominance for loop exit
>    set_loop(le, loop);
>