Request for reviews (S): 6880053

Tom Rodriguez Thomas.Rodriguez at Sun.COM
Wed Sep 9 16:59:36 PDT 2009


I suspected that was the purpose but TypeOopPtr::NOTNULL seems  
nonsensical to me.  Is that really a valid type?

tom

On Sep 9, 2009, at 4:43 PM, Vladimir Kozlov wrote:

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



More information about the hotspot-compiler-dev mailing list