review (S) for 6856025: assert(_base >= OopPtr && _base <= KlassPtr,"Not a Java pointer")

[Tom Rodriguez]

On Jun 30, 2009, at 2:49 PM, Vladimir Kozlov wrote:

> Tom Rodriguez wrote:
>> On Jun 30, 2009, at 2:33 PM, Vladimir Kozlov wrote:
>>> After sending this I don't see that you fixed store_oop_to_unknown.
>>> Is it intentional to use TypeInstPtr::BOTTOM as val_type in case  
>>> of narrow field/element?
>> The value_type for the prebarrier is declared to be TypeOopPtr  
>> since it bottoms out in LoadNode::make which always returns the  
>> wide type.  So TypeInstPtr::BOTTOM is the worst case type for the  
>> store_oop_to_unknown case.  Is there something wrong with that?
>
> Nothing wrong with it. I'm just wondering why you don't use more  
> precise
> type for array's element (instance's field type is fine) by using  
> make_oopptr().

I'm confused.  This whole chunk tries to pick the best type it can for  
store_oop_to_unknown.

   const TypeOopPtr* val_type = NULL;
   if (adr_type->isa_instptr()) {
     if (at->field() != NULL) {
       // known field.  This code is a copy of the do_put_xxx logic.
       ciField* field = at->field();
       if (!field->type()->is_loaded()) {
         val_type = TypeInstPtr::BOTTOM;
       } else {
         val_type = TypeOopPtr::make_from_klass(field->type()- 
 >as_klass());
       }
     }
   } else if (adr_type->isa_aryptr()) {
     val_type = adr_type->is_aryptr()->elem()->make_oopptr();
   }
   if (val_type == NULL) {
     val_type = TypeInstPtr::BOTTOM;
   }

What change are you suggesting?

tom

>
> Vladimir
>
>> tom
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> Vladimir Kozlov wrote:
>>>> Looks good.
>>>> Vladimir
>>>> Tom Rodriguez wrote:
>>>>> http://cr.openjdk.java.net/~never/6856025/