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

[Roman Kennke]

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

Yikes! I should not work late in the evening...

Updated:
Incremental:
http://cr.openjdk.java.net/~rkennke/JDK-8213615/webrev.03.diff/
Full:
http://cr.openjdk.java.net/~rkennke/JDK-8213615/webrev.03/


Good now?

Thanks for reviewing, Vladimir!

Roman

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20181115/e08127d5/signature.asc>