PATCH: using mixed types in MIN2/MAX2 functions
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Mon Jun 16 17:01:20 UTC 2014
Hi Dan,
I created a bug four this issue:
https://bugs.openjdk.java.net/browse/JDK-8046938
/Jesper
Dan Horák skrev 16/6/14 18:29:
> On Mon, 16 Jun 2014 17:26:24 +0200
> Jesper Wilhelmsson <jesper.wilhelmsson at oracle.com> wrote:
>
>> 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.
>
> I wasn't sure what would be better approach here
>
>> 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 for the comments, I'll prepare new webrev hopefully tomorrow
>
>
> Dan
>
>> 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