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