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

[Vladimir Kozlov]

On 6/19/18 9:01 AM, Vladimir Kozlov wrote:
> 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); }

Sorry, test should be reversed because it guarantee SP if call is *not* 
removed:

virtual bool        guaranteed_safepoint()  { return 
!(C->eliminate_boxing() && is_boxing_method() && 
(proj_out_or_null(TypeFunc::Parms) == NULL)); }

Vladimir

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