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

Jini George jini.george at oracle.com
Wed Nov 8 06:28:12 UTC 2017


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 ?

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