PATCH: using mixed types in MIN2/MAX2 functions

[Dan Horák]

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
> >>>>>>>>>>>
> >>>>>>>