Request for reviews (S): 6832293: JIT compiler got wrong result in type checking with -server

[Vladimir Kozlov]

Tom Rodriguez wrote:
> 
> On May 8, 2009, at 3:03 PM, Vladimir Kozlov wrote:
> 
>> Tom,
>>
>> With this you will have the same problem with 2- multi- dimensional 
>> arrays
>> which I try to avoid with my changes. But I agree, may be element klasses
>> should be used only for interface and subtype checks.
> 
> What problem is that?  base_element_klass is the instanceKlass at the 
> bottom of the objArray, irrespective of the arity of the array.

Sorry, I missed that you used base_element_klass() instead of element_klass().
Yes, your code will work. I will update changes.

> 
>> On other hand why we are not using p0->meet(p1) == p0 || == p1?
> 
> Something like that might do a better job without duplicating logic, 
> though I think we might need to handle interfaces explicitly anyway.

Yes, I tried and it does not work with interfaces.

Thanks,
Vladimir

> 
> tom
> 
> 
>>
>>
>> Vladimir
>>
>>
>> Tom Rodriguez wrote:
>>> That doesn't seem quite right to me or maybe I just don't like the 
>>> way it's phrased.  The rules for subtyping for arrays are different 
>>> than instances, so walking up the classes would change the subtype 
>>> test.  It also creates in consistency between klass and xklass.  What 
>>> about this:
>>> diff --git a/src/share/vm/opto/subnode.cpp 
>>> b/src/share/vm/opto/subnode.cpp
>>> --- a/src/share/vm/opto/subnode.cpp
>>> +++ b/src/share/vm/opto/subnode.cpp
>>> @@ -639,8 +639,11 @@ const Type *CmpPNode::sub( const Type *t
>>>     int kps = (p0->isa_klassptr()?1:0) + (p1->isa_klassptr()?1:0);
>>>     if (klass0 && klass1 &&
>>>         kps != 1 &&             // both or neither are klass pointers
>>> -        klass0->is_loaded() && !klass0->is_interface() && // do not 
>>> trust interfaces
>>> -        klass1->is_loaded() && !klass1->is_interface()) {
>>> +        // do not trust interfaces
>>> +        klass0->is_loaded() && !klass0->is_interface() &&
>>> +        (!klass0->is_obj_array_klass() || 
>>> !klass0->as_obj_array_klass()->base_element_klass()->is_interface()) &&
>>> +        klass1->is_loaded() && !klass1->is_interface() &&
>>> +        (!klass1->is_obj_array_klass() || 
>>> !klass1->as_obj_array_klass()->base_element_klass()->is_interface())) {
>>>       bool unrelated_classes = false;
>>>       // See if neither subclasses the other, or if the class on top
>>>       // is precise.  In either of these cases, the compare is known
>>> tom
>>> On May 8, 2009, at 1:12 PM, Vladimir Kozlov wrote:
>>>> http://cr.openjdk.java.net/~kvn/6832293/webrev.00
>>>>
>>>> Fixed 6832293: JIT compiler got wrong result in type checking with 
>>>> -server
>>>>
>>>> Problem:
>>>> The code in CmpPNode::sub is broken for arrays of interface types.
>>>>
>>>> Solution:
>>>> In CmpPNode::sub compare klasses of obj arrays elements..
>>>>
>>>> Reviewed by:
>>>>
>>>> Fix verified (y/n): y, modified (for jdk7) bug's test
>>>>
>>>> Other testing:
>>>> JPRT
>>>>
>