PATCH: using mixed types in MIN2/MAX2 functions
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Mon Jun 16 15:26:24 UTC 2014
Hi Dan,
Since you got a comment from Bengt maybe you should fix that issue before I
upload the new webrev.
I looked through the webrev and I have one additional comment. I would prefer if
the changes in arguments.cpp was the other way, casting stuff to uintx instead
of size_t, since we use the result of MAX and MIN to set flags of type uintx.
When I applied the webrev a change in
src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp got in the way of the
patch. The patch still applies but with an offset, so patch complains. If you
build a new webrev with the changes above, also pull down the latest hs-gc to
get the g1CollectedHeap.cpp change.
Thanks!
/Jesper
Dan Horák skrev 16/6/14 16:21:
> On Mon, 16 Jun 2014 16:19:17 +0200
> Jesper Wilhelmsson <jesper.wilhelmsson at oracle.com> wrote:
>
>> Dan,
>>
>> I can host the webrev. Send me the zipped webrev archive and I'll
>> upload it to cr.openjdk.
>
> Jesper, thanks a lot, the whole archive is at
> http://fedora.danny.cz/openjdk/size_t-1/webrev.zip
>
>
> Dan
>
>> /Jesper
>>
>> David Holmes skrev 16/6/14 13:15:
>>> On 16/06/2014 5:17 PM, Dan Horák wrote:
>>>> On Mon, 16 Jun 2014 16:31:29 +1000
>>>> David Holmes <david.holmes at oracle.com> wrote:
>>>>
>>>>> On 13/06/2014 10:55 PM, Dan Horák wrote:
>>>>>> On Fri, 13 Jun 2014 14:10:53 +0200
>>>>>> Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
>>>>>>> Then a formality comment. I think you have to post the webrev on
>>>>>>> cr.openjdk.java.net to get the legal stuff correct.
>>>>>>
>>>>>> hm, I've used this procedure (my webrev, then someone privileged
>>>>>> takes it and pushes it) few times in the past and it worked.
>>>>>
>>>>> We've been advised that all contributions have to come in via
>>>>> OpenJDK infrastructure - either a webrev on cr.openjdk.java.net
>>>>> or as attachments to mailing list mails.
>>>>
>>>> I understand, but AFAIK cr.openjdk.java.net is accessible only to
>>>> people with push access, which I'm not.
>>>
>>> No you only need Author status, not Committer.
>>>
>>>> So the question is will the list
>>>> accept 3MB attachment? It is the size of the zipped webrev. Or
>>>> will a plain text patch suffice?
>>>
>>> For large webrevs you may be able to find someone to host it for
>>> you on cr.openjdk. I don't know what limit exists on the mailing
>>> list but I'd hate to see very large patches submitted that way.
>>>
>>> Cheers,
>>> David
>>>
>>>
>>>>
>>>> Dan
>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>>> 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
>>>>>>>>>>>
>>>>>>>
More information about the hotspot-dev
mailing list