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

[David Holmes]

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