PATCH: using mixed types in MIN2/MAX2 functions

[Bengt Rutisson]

Hi Dan,

I haven't looked in detail at this fix, but I have a question. How do we 
make sure that we don't introduce new calls to MAX2() that mix size_t 
and uintx? It seems like this fix is a bit fragile in that sense.

What do you think about introducing specialized MAX2() definitions for 
the size_t and uintx combinations. That would avoid the casting in the 
code and will be safer going forward.

One detail also. Why does *hum_bytes have to be cast to size_t here? It 
is already a size_t*.
http://fedora.danny.cz/openjdk/size_t-1/webrev/src/share/vm/gc_implementation/g1/concurrentMark.cpp.udiff.html

Then a formality comment. I think you have to post the webrev on 
cr.openjdk.java.net to get the legal stuff correct.

Thanks,
Bengt


On 2014-06-13 09:55, Dan Horák wrote:
> On Thu, 12 Jun 2014 21:49:40 +0200
> Jesper Wilhelmsson <jesper.wilhelmsson at oracle.com> wrote:
>
>> Hi,
>>
>> Which repository is the patch built on? Since it is mostly GC code it
>> would be good if it goes in through jdk9/hs-gc.
>>
>> The changes looks good in the webrev, but the patch did not apply to
>> a recent jdk9/hs-gc. Especially the collector policy code has changed
>> a lot recently.
>>
>> I would prefer to look at the changes applied in my workspace before
>> approving it, so if you could update to a recent hs-gc it would be
>> great.
> I have refreshed the patch for the hs-gc repository, please see
> http://fedora.danny.cz/openjdk/size_t-1/webrev/
>
>
> 	Thanks,
>
> 		Dan
>   
>> Thanks!
>> /Jesper
>>
>> Vladimir Kozlov skrev 12/6/14 20:57:
>>> Thank you, Dan
>>>
>>> I think casting is acceptable solution. Someone in GC group should
>>> review and sponsor this since you touched GC code mostly.
>>>
>>> Regards,
>>> Vladimir
>>>
>>> On 6/12/14 2:05 AM, Dan Horák wrote:
>>>> Hello,
>>>>
>>>> this the last and largest part of my changes required on s390
>>>> (32-bit) to build OpenJDK out of the box. The MIN2/MAX2 functions
>>>> are implemented using templates and require both arguments to be
>>>> of the same type. This is a problem when one parameter is of
>>>> size_t type and the second of uintx type and the platform has
>>>> size_t defined as eg. unsigned long as on s390 (32-bit).
>>>>
>>>> As a solution [1] I chose to typecast the smaller type (uintx) to
>>>> size_t which is of same or larger size.
>>>>
>>>> [1] http://fedora.danny.cz/openjdk/size_t/webrev/
>>>>
>>>> Note: I'm Red Hat employee (dhorak at redhat.com), so I should be
>>>> covered by Red Hat's CLA for my contributions.
>>>>
>>>>
>>>>      With regards
>>>>
>>>>          Dan
>>>>