Request for review: 7132690 InstanceKlass:_reference_type should be u1 type

[Jiangli Zhou]

Hi Dean,

I was looking for existing code for hotspot conventions. My quick search 
didn't reveal any. Thanks for the pointer.

Thanks,

Jiangli

Dean Long wrote:
> I agree with Vitaly.  You can find existing examples in the code:
>
>   interpreter/rewriter.cpp:        assert(cache_index == 
> (u1)cache_index, "index overflow");
>
> You could also assert that reference_type == t after the assignment.
>
> dl
>
> On 1/24/2012 7:19 PM, Jiangli Zhou wrote:
>> Hi Vitaly,
>>
>> Thanks for catching it! A range check seems to be more explicit in 
>> this case:
>>
>>   http://cr.openjdk.java.net/~jiangli/7132690/webrev.02/
>>
>> Thanks,
>>
>> Jiangli
>>
>> On 01/24/2012 04:17 PM, Vitaly Davidovich wrote:
>>>
>>> I think you'll be fully covered (including negative values) by doing 
>>> the following instead of hard coding max value in the assert:
>>>
>>> assert(t == (u1)t, "doesn't fit")
>>>
>>> Sent from my phone
>>>
>>> On Jan 24, 2012 6:33 PM, "Jiangli Zhou" <jiangli.zhou at oracle.com 
>>> <mailto:jiangli.zhou at oracle.com>> wrote:
>>>
>>>     The updated webrev is :
>>>     http://cr.openjdk.java.net/~jiangli/7132690/webrev.01/
>>>     <http://cr.openjdk.java.net/%7Ejiangli/7132690/webrev.01/>
>>>
>>>     Thanks,
>>>     Jiangli
>>>
>>>     On 01/24/2012 02:54 PM, Jiangli Zhou wrote:
>>>>     Hi Vitaly,
>>>>
>>>>     An assert in the setter probably is a good idea. I should have
>>>>     added it when making the change. Thanks for the comments!
>>>>
>>>>     Thanks,
>>>>     Jiangli
>>>>
>>>>     On 01/24/2012 02:13 PM, Vitaly Davidovich wrote:
>>>>>
>>>>>     Hi Jiangli,
>>>>>
>>>>>     This is probably overly paranoid so feel free to ignore, but
>>>>>     should the setter in InstanceKlass assert that the passed in
>>>>>     ReferenceType fits into a u1 instead of silently narrowing it?
>>>>>     Or change the setter to take a u1 and make caller do the cast?
>>>>>     This would prevent someone defining another member of the enum
>>>>>     with an explicit value that doesn't fit into u1.  Like I said,
>>>>>     paranoia ... :)
>>>>>
>>>>>     Thanks
>>>>>
>>>>>     Sent from my phone
>>>>>
>>>>>     On Jan 24, 2012 3:06 PM, "Jiangli Zhou"
>>>>>     <jiangli.zhou at oracle.com <mailto:jiangli.zhou at oracle.com>> wrote:
>>>>>
>>>>>         Hi,
>>>>>
>>>>>         Please review the change for 7132690:
>>>>>
>>>>>          http://cr.openjdk.java.net/~jiangli/7132690/webrev.00/
>>>>>         <http://cr.openjdk.java.net/%7Ejiangli/7132690/webrev.00/>
>>>>>
>>>>>         It changes InstanceKlass::_reference_type from
>>>>>         ReferenceType to u1. The ReferenceType is defined as an
>>>>>         enum with 6 values
>>>>>         (src/share/vm/utilities/globalDefinitions.hpp). The change
>>>>>         saves 4-byte for each class.
>>>>>
>>>>>         Tested with runThese and ute vm.quick.testlist. Ran jprt.
>>>>>
>>>>>         Thanks,
>>>>>
>>>>>         Jiangli
>>>>>
>>>>
>>>
>>