RFR (XXS) CR 8006997: ContendedPaddingWidth should be uintx

[David Holmes]

On 7/02/2013 2:47 AM, Vladimir Kozlov wrote:
> On 2/6/13 5:33 AM, Aleksey Shipilev wrote:
>> Hi David,
>>
>> On 02/05/2013 10:00 AM, David Holmes wrote:
>>> You also need to change all the expressions using this value to use
>>> unsigned types and update all the printfs to use an unsigned format
>>> specifier.
>>
>> Ah, yes, thanks!
>>
>>> > grepsrc.sh ContendedPaddingWidth
>>> ./cpu/sparc/vm/vm_version_sparc.cpp: (cache_line_size >
>>> ContendedPaddingWidth))
>>
>> cache_line_size is signed (although semantically fits into unsigned)
>> What's the usual convention around HotSpot to handle signed/unsigned
>> comparisons like these? Use static_cast?
>>
>> if (static_cast<unsigned int>(cache_line_size) > ContededPaddingWidth)

This is rarely ever used in hotspot. If you see one it is from someone 
relatively new to hotspot :)

>>
>> Use the ordinary cast:
>>
>> if ((unsigned int)cache_line_size) > ContededPaddingWidth)
>>
>> ...or even push forward changing all the cache_line_size family to
>> unsigned?
>>
>> Grepping the code, ordinary cast seems to be used the most. I can do
>> either of three. Thoughts?
>
> Ordinary cast is fine, and you can change cache_line_size type if you
> want (it is used only in few places). One note, we have internal type
> 'uint' defined for such cases.

Grepping for cache_line_size I see a number of different uses and a mix 
of signed and unsigned definitions for it. But changing to uint would 
have a flow on affect to  prefetch_data_size() which is signed too.

I have to wonder whether leaving ContendedPaddingWidth as signed is not 
the simplest thing to do as it may be impractical, if not impossible, to 
get uniformity of the signed-ness of expressions across all platforms.

David
-----


>
> Thanks,
> Vladimir
>
>>
>> -Aleksey.
>>