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

[Tom Rodriguez]

Oh yeah, that's right.  I hadn't fixed it before but the one I sent  
our for review has it.  Sorry.

tom

On Jun 30, 2009, at 3:13 PM, Vladimir Kozlov wrote:

> Sorry, in the original webrev (webrev/g1) which you sent me
> there were no changes in graphKit.cpp so I assumed you missed it.
> But I see now in the final changes you have it.
>
> Sorry for confusion.
>
> Thanks,
> Vladimir
>
> Tom Rodriguez wrote:
>> 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/