RFR: JDK-8213615: GC/C2 abstraction for escape analysis

[Vladimir Kozlov]

On 11/14/18 12:59 PM, Roman Kennke wrote:
> Hi Vladimir,
> 
>> G1 and ZGC code refactoring matches previous code.
>> add_to_congraph_unsafe_access() is also good.
> 
> OK.
> 
>> Why add_final_edges_unsafe_access() does not include 'if (opcode ==
>> Op_GetAndSetP' code? You broke sequence where (adr_type == NULL) should
>> be checked first.
> 
> Hmm, right. The idea was that Sheanndoah would need to call this for its
> own set of CAS nodes, we'd handle the first part separately anyway, and
> would therefore not be needed in general path:
> 
>   if (opcode == Op_GetAndSetP || opcode == Op_GetAndSetN ||
>        opcode == Op_CompareAndExchangeN || opcode ==
> Op_CompareAndExchangeP) {
>      add_local_var_and_edge(n, PointsToNode::NoEscape, adr, NULL);
>    }
> 
> However, since it's guarded by the if (opcode== ..) it doesn't hurt to
> leave it there. I reinstated the original sequence.

You forgot to remove this code from main switch after copying it to add_final_edges_unsafe_access().

> 
>> Also fail check is wrong. Should be:
>>
>>         if (add_final_edges_unsafe_access(n, opcode)) {
>>           break;
>>         }
>>         ELSE_FAIL("Op_StoreP");
> 
> Right. Fixed.
> 
>> Why you moved record_for_optimizer() to .cpp file?
> 
> This is because BarrierSetC2 impls would now include escape.hpp but not
> necessarily phaseX.hpp, or at least not necessarily phaseX.hpp before
> escape.hpp. And I did not want to drag this include into escape.hpp
> (which would be correct if the method is using a PhaseIGVN method).
> Instead, I moved the body into escape.cpp and we have the correct
> include there already.

Okay. Good to know.

Thanks,
Vladimir

> 
> Updated webrevs:
> Incremental:
> http://cr.openjdk.java.net/~rkennke/JDK-8213615/webrev.02.diff/
> Full:
> http://cr.openjdk.java.net/~rkennke/JDK-8213615/webrev.02/
> 
> Better?
> 
> Thanks a lot for reviewing!
> Roman
> 
> 
>> Thanks,
>> Vladimir
>>
>> On 11/13/18 9:05 AM, Roman Kennke wrote:
>>> Sure no problem. Thank you!
>>>
>>> Roman
>>>
>>>
>>>> I have to review it in details. Give me some time.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 11/13/18 4:24 AM, Roman Kennke wrote:
>>>>> Thanks Roland for the review!
>>>>>
>>>>> Vladimir: do you want to take a deeper look too? Or can I consider your
>>>>> 'I like the proposal from JIT POV' as 'reviewed' ? ;-)
>>>>>
>>>>> Roman
>>>>>
>>>>>>> http://cr.openjdk.java.net/~rkennke/JDK-8213615/webrev.01/
>>>>>>
>>>>>> Looks ok to me.
>>>>>>
>>>>>> Roland.
>>>>>>
>>>>>
>>>
>