Request for reviews (S): 6880053
Vladimir Kozlov
Vladimir.Kozlov at Sun.COM
Wed Sep 9 17:32:13 PDT 2009
It is valid C2 type since all type methods (xmeet) process it
but it should not exist from java point of view. And I'm using it
with assumption that none java programs use such type.
I am also not fully comfortable with it but I can't find
other type which could be used (I tried).
I talked with John and he agreed with this change.
I ran CTW and it did not find problems with this change.
But we may still hit some problems where we would expect
a real java type and will get this one. But such cases
should be caught by asserts we have.
The other solution (which we rejected before) is to modify
oop map build to list raw pointers which have CheckCastPP
users. Then we will not need the second CheckCastPP.
Vladimir
Tom Rodriguez wrote:
> 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