Hi Magnus, thanks for the review!
I haven't received a review for the hotspot source changes yet, so I will have to wait. Regards, Jakub On 2019-01-23 at 13:55 +0100, Magnus Ihse Bursie wrote: > Hi Jakub, > > On 2019-01-15 17:31, Jakub Vaněk wrote: > > Hi Magnus and Erik, > > > > I have added the link to the repository to README and I have > > removed > > the link to the mailing list thread. I have also recreated the > > GitHub > > repository. Now it is a fork of the mentioned repository with two > > extra > > commits containing README and the build scripts. > > > > New webrev URL: > > http://cr.openjdk.java.net/~jakvanek/8215902/webrev.04/ > > Bug: https://bugs.openjdk.java.net/browse/JDK-8215902 > > Sorry for the late reply. > > This looks very good! Thank you for fixing this, including rebasing > the > github repo. > > I'm not sure if you've gotten reviews from the hotspot team for the > hotspot source changes, but from a build perspective, this is good to > go. > > /Magnus > > > > Regards, > > > > Jakub > > > > On 2019-01-15 at 15:05 +0100, Magnus Ihse Bursie wrote: > > > On 2018-12-25 16:19, Jakub Vaněk wrote: > > > > Hi, > > > > > > > > please review this webrev. It is a successor of the softfloat-3 > > > > [patch] > > > > thread (first email > > > > > > > > http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2018-November/031311.html > > > > ) > > > > > > > > Changes since the last patch (v6): > > > > > > > > - renamed --with-softloat* to --with-sflt* (it is more compact > > > > and > > > > it > > > > corresponds to the old --with-sflt-lib=... option) > > > > > > > > - license is now obtained via --with-sflt-license switch (so it > > > > is > > > > not > > > > included in OpenJDK source tree) > > > > > > > > - updated documentation (slight rewording, added the license > > > > option) > > > > > > > > - checks for default --with/--without behavior are in place > > > > again > > > > (I forgot them when I changed the way the library is > > > > detected) > > > > > > > > - added a simple testcase - I found a disrepancy between > > > > softfloat > > > > and > > > > system function behavior. When a float with bits 0x003FFFFF > > > > is > > > > added to 0x00000001, the correct result is 0x00400000, but > > > > the > > > > default software floating point implementation returns > > > > 0x00000000. > > > > However I'm not sure where to put this test - now it is in > > > > test/hotspot/jtreg/compiler/floatingpoint. > > > > > > > > - comments in code refer to CR 6757269 and newly JDK-8215902 > > > > too. > > > > > > > > I have created a repository with SoftFloat-3e with build > > > > configuration > > > > specifically for OpenJDK on armel: > > > > https://github.com/ev3dev-lang-java/softfloat-openjdk > > > > > > > > I can add a link to it to the documentation. > > > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8215902 > > > > Webrev: http://cr.openjdk.java.net/~jakvanek/8215902/webrev.02/ > > > > > > Hi Jakub, > > > > > > In general this looks good. > > > > > > Some comments: > > > > > > I agree with Erik that you can add a link to your github project; > > > compiling SoftFloat is outside the scope of the OpenJDK build > > > instructions, but it can sure be helpful to lower the bar for > > > users > > > wanting to do that. Just one question: any particular reason you > > > didn't > > > create your github repo by forking the official > > > https://github.com/ucb-bar/berkeley-softfloat-3? That way, it > > > would > > > have > > > been easy for users to see that you were not adding any malicious > > > or > > > suspicious code to the original SoftFloat distribution. > > > > > > On the other hand, I think the link to > > > > > > > http://mail.openjdk.java.net/pipermail/aarch32-port-dev/2016-November/000611.html > > > > > > is unnecessary and just creates clutter in the documentation. > > > Please > > > remove it. > > > > > > /Magnus > > > > CI build: > > > > https://ci.adoptopenjdk.net/view/ev3dev/job/openjdk12_build_ev3_linux/67/ > > > > > > > > Cheers, > > > > > > > > Jakub > > > > > >
