RFR(S): 8205107: assert(c->Opcode() == Op_SafePoint) failed: broken outer loop

[Vladimir Kozlov]

On 6/19/18 7:22 AM, Nils Eliasson wrote:
> I looked at it in a bit more detail.
> 
> The call is a java/lang/Integer::valueOf that is considered a macro node 
> so it is kept around until macro expansion which happens after the place 
> where we hit the assert. So the call will go away.

I assume you are talking about eliminate_boxing_node() in macro expansion.

> 
> However SafepointNode::Identity does check:
> 
> if( n0->is_Call() && n0->as_Call()->guaranteed_safepoint() ) {
>        // Useless Safepoint, so remove it
> 
> And CallNode::guaranteed_safepoint() is:
> 
>    // Are we guaranteed that this node is a safepoint?  Not true for 
> leaf calls and
>    // for some macro nodes whose expansion does not have a safepoint on 
> the fast path.
>    virtual bool        guaranteed_safepoint()  { return true; }
> 
> No check for call/macro-nodes is implemeted despite the comment! If I 
> fix this the problem goes away.

It returns false for Allocate nodes which are macro nodes. That is what 
comment says.

In case of boxing node I think we should add to CallStaticJavaNode class

   // Boxing call which is not used and will be removed.
   virtual bool        guaranteed_safepoint()  { return 
C->eliminate_boxing() && is_boxing_method() && 
(proj_out_or_null(TypeFunc::Parms) == NULL); }

Vladimir

> 
> // Nils
> 
> 
> 
> On 2018-06-18 15:41, Roland Westrelin wrote:
>>> Roland, why we need to keep Safepoint in Outer loop if we have call?
>> I double checked with a simple test case and we don't keep the safepoint
>> if there's a call so it's indeed strange that we would need Nils' patch.
>>
>> Roland.
>