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

[Tom Rodriguez]

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.

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

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