RFR: SA: JDK-8189798: SA cleanup - part 1

Jini George jini.george at oracle.com
Wed Nov 8 06:31:50 UTC 2017


Ok, Thank you!

-Jini.

On 11/8/2017 12:00 PM, David Holmes wrote:
> Hi Jini,
> 
> On 8/11/2017 4:28 PM, Jini George wrote:
>> Hi David,
>>
>> If we don't retain the cast, wouldn't that mean that we would be 
>> comparing 2 64 bit values in a 64 bit environment which would not be 
>> the intended comparison ?
> 
> Yes. I was pointing out the reason for the cast.
> 
> David
> 
>> Thanks,
>> Jini.
>>
>> On 11/8/2017 7:49 AM, David Holmes wrote:
>>> Hi Jini,
>>>
>>>> On 11/7/17 4:16 AM, Jini George wrote:
>>>>> Thank you very much, Coleen, for the review. My answers inline:
>>>>>
>>>>> On 11/2/2017 5:09 PM, coleen.phillimore at oracle.com wrote:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~jgeorge/8189798/webrev.00/src/hotspot/share/runtime/stackValue.hpp.udiff.html 
>>>>>>
>>>>>>
>>>>>> + return (*(int *)&_integer_value == *(int *)&value->_integer_value);
>>>>>>
>>>>>>
>>>>>> I don't think the *(int*) casts for _integer_value are needed in 
>>>>>> these files.  Can you remove them?
>>>>>
>>>>> [Jini] I think since _integer_value is of type intptr_t (which 
>>>>> could be 8 or 4 bytes long depending on the data model), the 
>>>>> removal of the casts could result in an incorrect comparison 
>>>>> (mostly for the 64 bit environment). Let me know if you disagree.
>>>
>>> You're comparing two _integer_value fields that are both intptr_t. 
>>> The important part you've overlooked is the comment preceding this:
>>>
>>>         // [phh] compare only low addressed portions of intptr_t slots
>>> -      return (*(int *)&_i == *(int *)&value->_i);
>>> +      return (*(int *)&_integer_value == *(int 
>>> *)&value->_integer_value);
>>>
>>> For some we reason although intptr_t we're only interested in the 
>>> lower 32-bits. I have no idea why, nor why we would truncate the 
>>> value when printing.
>>>
>>> David
>>> -----


More information about the hotspot-dev mailing list