Request for reviews (S): 6880053

[Vladimir Kozlov]

As you remember we have 2 CheckCastPP nodes in clone() intrinsic.
One is original which we move and pin below arraycopy call
to guaranty that all memory accesses to the cloned object
are placed below arraycopy.
And the second is used to have a live oop across arraycopy call.
If both have the same type (bug's case) the CheckCastPPNode::identity()
will eliminate the original CheckCastPP since it references the second:

   // Replace raw memory edge with new CheckCastPP to have a live oop
   // at safepoints instead of raw value.
   assert(new_obj->is_CheckCastPP() && new_obj->in(1) == alloc_obj->in(1), "sanity");
   alloc_obj->set_req(1, new_obj);

Vladimir

Tom Rodriguez wrote:
> I'm don't understand this.  What is TypeOopPtr::NOTNULL supposed to 
> represent?  That doesn't seem like a valid type in the lattice since it 
> would have to correspond to Object:NotNull which is already represented 
> by TypeInstPtr::NOTNULL.  Why will the graph be incorrect if the type is 
> TypeInstPtr::NOTNULL?
> 
> tom
> 
> On Sep 9, 2009, at 3:35 PM, Vladimir Kozlov wrote:
> 
>> http://cr.openjdk.java.net/~kvn/6880053/webrev.00
>>
>> Fixed 6880053: assert(alloc_obj->as_CheckCastPP()->type() != 
>> TypeInstPtr::NOTNULL)
>>
>> Problem:
>> I added this assert in 6875577 fix to catch cases when
>> the type of the cloned object is TypeInstPtr::NOTNULL
>> since it will produce incorrect graph.
>> And Nightly testing found such case: the method
>> sun.reflect.GeneratedMethodAccessor4.invoke(Object o, Object[] ao)
>> returns the clone of the Object o argument.
>>
>> Solution:
>> Use more general oop type TypeOopPtr::NOTNULL for
>> additional CheckCastPP node in clone() instrinsic and arraycopy.
>> Also fix instance_id meet for TypeOopPtr.
>>
>> Reviewed by:
>>
>> Fix verified (y/n): y, test
>>
>> Other testing:
>> JPRT
>>
>