RFR(M): 8130847: Cloned object's fields observed as null after C2 escape analysis

[Roland Westrelin]

Updated webrev with Vladimir’s comments:

http://cr.openjdk.java.net/~roland/8130847/webrev.01/

Roland.

> On Jul 30, 2015, at 2:48 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> On 7/29/15 6:57 AM, Roland Westrelin wrote:
>>> The next change puzzles me:
>>> 
>>> -         if (!call->may_modify(tinst, phase)) {
>>> +         if (call->may_modify(tinst, phase)) {
>>> -           mem = call->in(TypeFunc::Memory);
>>> +           assert(call->is_ArrayCopy(), "ArrayCopy is the only call node that doesn't make allocation escape");
>>> 
>>> Why only ArrayCopy? I think it is most of calls. What set of tests you ran?
>>> 
>>> Methods naming is confusing. membar_for_arraycopy() does not check for membar but for calls which can modify. handle_arraycopy() could be make_arraycopy_load().
>> 
>> What about:
>> 
>> static bool may_modify(const TypeOopPtr *t_oop, MemBarNode* mb, PhaseTransform *phase);
>> 
>> instead of membar_for_arraycopy()
>> 
>> So ArrayCopyNode would have:
>> 
>> virtual bool may_modify(const TypeOopPtr *t_oop, PhaseTransform *phase);
>> 
>> and
>> 
>> static bool may_modify(const TypeOopPtr *t_oop, MemBarNode* mb, PhaseTransform *phase);
>> 
>> that do the same thing except the static method also looks for a graph pattern starting from a MemBar.
> 
> Yes, it is better.
> 
> Thanks,
> Vladimir
> 
>> 
>> Roland.
>> 
>>> 
>>> Add explicit check:
>>> && strcmp(_name, "unsafe_arraycopy") != 0)
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> On 7/28/15 7:05 AM, Roland Westrelin wrote:
>>>> http://cr.openjdk.java.net/~roland/8130847/webrev.00/
>>>> 
>>>> When an allocation which is the destination of an ArrayCopyNode is eliminated, field’s values recorded at a safepoint (to reallocate the object) do not take the ArrayCopyNode into account at all and the effect or the ArrayCopyNode is lost on a deoptimization. This fix records values from the source of the ArrayCopyNode, emitting new loads if necessary.
>>>> 
>>>> I also use the opportunity to pin the loads generated in LoadNode::can_see_arraycopy_value() because they depend on all checks that validate the array copy and not only on the check that immediately dominates.
>>>> 
>>>> Roland.