RFR(M)(round 2): 8215902: Add support for SoftFloat-3e library

Jakub Vaněk linuxtardis at gmail.com
Thu Jan 24 23:17:38 UTC 2019


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




More information about the build-dev mailing list