[PATCH v6] Add support for SoftFloat library on ARM

Fri Dec 14 20:33:18 UTC 2018

Hi Erik,

thanks for the pointer - it is clearer to me now. Yup, in OpenJDK 10,
autoconf option frontend sets up a FREETYPE_LICENSE variable and that
file is then copied to the legal directory in its own copy target.

Jakub

On 2018-12-14 at 12:19 -0800, Erik Joelsson wrote:
> Note that since we moved freetype source into the repo, that
> handling 
> seems to have gone away. Look at how it was done before JDK-8193017. 
> Basically we had another configure parameter to point to the license
> file.
> 
> /Erik
> 
> On 2018-12-14 12:11, Magnus Ihse Bursie wrote:
> > Have a look at how we are handling the freetype license in lib-
> > freetype.m4. This sounds like a similar scenario.
> > 
> > /Magnus
> > 
> > > 14 dec. 2018 kl. 21:05 skrev Jakub Vaněk <linuxtardis at gmail.com>:
> > > 
> > > Hi Boris,
> > > 
> > > thanks for the info. I will remove it from the patch.
> > > 
> > > However, I'm not sure if I can then distribute the resulting
> > > OpenJDK
> > > build straight out of images directory.
> > >  From the softfloat license: "Redistributions in binary form must
> > > reproduce the above copyright notice, this list of conditions,
> > > and the
> > > following disclaimer in the documentation and/or other materials
> > > provided with the distribution."
> > > 
> > > On the other hand, I think that if I put the license file in an
> > > archive
> > > alongside JDK legal files externally, I am still distributing the
> > > notice. This way the license won't need to be distributed with
> > > OpenJDK.
> > > 
> > > Cheers,
> > > 
> > > Jakub
> > > 
> > > > On 2018-12-14 at 17:47 +0300, Boris Ulasevich wrote:
> > > > Hi Jakub,
> > > > 
> > > > I was in doubt about SoftFloat licence if it is correct to add
> > > > it
> > > > to OpenJDK, so I consulted to Oracle, and answer is that
> > > > license (mark down) file should not be in the OpenJDK sources
> > > > because library codes are not included in the OpenJDK sources.
> > > > So please remove softfloat.md.
> > > > 
> > > > Thanks,
> > > > Boris
> > > > 
> > > > > On 13.12.2018 16:39, Jakub Vaněk wrote:
> > > > > Hmm, the patch still fails. I will have to redo the changes
> > > > > in the source tree and then reexport the patch. Editing
> > > > > the patch file manually wasn't a good idea.
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > Jakub
> > > > > 
> > > > > čt 13. 12. 2018 v 14:21 odesílatel Jakub Vaněk <
> > > > > linuxtardis at gmail.com> napsal:
> > > > > > Hi,
> > > > > > 
> > > > > > I have made a mistake when editing the patch (I forgot to
> > > > > > change
> > > > > > the
> > > > > > added line count in one header).
> > > 
> > > 
https://raw.githubusercontent.com/ev3dev-lang-java/openjdk-ev3/52d38ea739fda826759f171313991717e477c31d/upstream/softfloat.patch
> > > > > > Thanks,
> > > > > > 
> > > > > > Jakub
> > > > > > 
> > > > > > čt 13. 12. 2018 v 14:10 odesílatel Jakub Vaněk <
> > > > > > linuxtardis at gmail.com> napsal:
> > > > > > > Hi Boris,
> > > > > > > 
> > > > > > > I have updated the patch:
> > > 
> > > 
https://raw.githubusercontent.com/ev3dev-lang-java/openjdk-ev3/0887015e0dfcc45ffed03872a8ad42a609496459/upstream/softfloat.patch
> > > > > > > Now reinterpret_cast is used and the URL is present
> > > > > > > only in the documentation for enabling flags.
> > > > > > > 
> > > > > > > I didn't expect a performance penalty, I have seen
> > > > > > > somewhere that compilers are nowadays good in
> > > > > > > seeing through this type of operation:
> > > > > > > https://blog.regehr.org/archives/959
> > > > > > > 
> > > > > > > I tried compiling a similar code (with noinline
> > > > > > > functions)
> > > > > > > on https://godbolt.org/ and on GCC >= 4.6.4 memcpy is
> > > > > > > optimized away even on -O0.
> > > > > > > 
> > > > > > > However I agree that the reinterpret_cast way is more
> > > > > > > readable and conscise.
> > > > > > > 
> > > > > > > Thanks,
> > > > > > > 
> > > > > > > Jakub
> > > > > > > 
> > > > > > > čt 13. 12. 2018 v 9:11 odesílatel Boris Ulasevich
> > > > > > > <boris.ulasevich at bell-sw.com> napsal:
> > > > > > > > Hi Jakub,
> > > > > > > > 
> > > > > > > > Please see comments inline.
> > > > > > > > And one more point: please remove links to mail threads
> > > > > > > > from
> > > > > > > > sources (
> > > > > > > > 
http://mail.openjdk.java.net/pipermail/aarch32-port-dev/2016-November/000611.html
> > > > > > > > ).
> > > > > > > > 
> > > > > > > > Boris
> > > > > > > > 
> > > > > > > > 12.12.2018 17:36, Jakub Vaněk пишет:
> > > > > > > > > Hi Erik,
> > > > > > > > > 
> > > > > > > > > > On 2018-12-12 at 15:41 +0300, Boris Ulasevich
> > > > > > > > > > wrote:
> > > > > > > > > > Hi Jakub,
> > > > > > > > > > 
> > > > > > > > > > I do not understand why you use memcpy in
> > > > > > > > > > softfloat_arm.cpp.
> > > > > > > > > > If type cast is the only reason why don't you use
> > > > > > > > > > reinterpret_cast?
> > > > > > > > > > 
> > > > > > > > > > regards,
> > > > > > > > > > Boris
> > > > > > > > > 
> > > > > > > > > I'm using memcpy there because float32_t is a struct
> > > > > > > > > containing a
> > > > > > > > > single uint32_t member. I think that the proper way
> > > > > > > > > of
> > > > > > > > > doing a byte-
> > > > > > > > > level conversion between different types (type
> > > > > > > > > punning) is
> > > > > > > > > via memcpy.
> > > > > > > > 
> > > > > > > > Do not you expect performance penalty using type
> > > > > > > > conversion
> > > > > > > > this way?
> > > > > > > > > If I used a casted pointer, this would violate the
> > > > > > > > > rule
> > > > > > > > > that I can
> > > > > > > > > access a memory location through a pointer of only
> > > > > > > > > one
> > > > > > > > > type.
> > > > > > > > 
> > > > > > > > -fno-strict-aliasing gcc option is used to disable
> > > > > > > > compiler
> > > > > > > > complains on
> > > > > > > > this rule:
> > > > > > > > 
> > > > > > > > jdk-
> > > > > > > > jdk/make/hotspot/lib/JvmFeatures.gmk:  JVM_LDFLAGS_FEAT
> > > > > > > > URES
> > > > > > > > +=
> > > > > > > > -fno-strict-aliasing
> > > > > > > > 
> > > > > > > > > Using
> > > > > > > > > union for this seems to be also undefined in C++, as
> > > > > > > > > then I
> > > > > > > > > would
> > > > > > > > > access the value through a non-active member.
> > > > > > > > > 
> > > > > > > > > Thanks,
> > > > > > > > > 
> > > > > > > > > Jakub
> > > > > > > > > 
> > > > > > > > > > > On 10.12.2018 23:23, Jakub Vaněk wrote:
> > > > > > > > > > > Hi Erik,
> > > > > > > > > > > 
> > > > > > > > > > > > On 2018-12-10 at 09:39 -0800, Erik Joelsson
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > The build changes look ok to me.
> > > > > > > > > > > > 
> > > > > > > > > > > > I do think --enable-softfloat is redundant as
> > > > > > > > > > > > using
> > > > > > > > > > > > any of the
> > > > > > > > > > > > other
> > > > > > > > > > > > parameters would imply it to be set, but it's
> > > > > > > > > > > > ok to
> > > > > > > > > > > > have it
> > > > > > > > > > > > there.
> > > > > > > > > > > 
> > > > > > > > > > > Thanks for the review. The motivation for this
> > > > > > > > > > > was that
> > > > > > > > > > > I was
> > > > > > > > > > > closely
> > > > > > > > > > > following how libffi is handled. The enable
> > > > > > > > > > > option was
> > > > > > > > > > > a workaround
> > > > > > > > > > > for
> > > > > > > > > > > the detection of softfloat in system paths. I
> > > > > > > > > > > don't
> > > > > > > > > > > think this is
> > > > > > > > > > > how
> > > > > > > > > > > the library is going to be used, so I removed
> > > > > > > > > > > this
> > > > > > > > > > > option.
> > > > > > > > > > > 
> > > > > > > > > > > New patch can be found here:
> > > 
> > > 
https://github.com/ev3dev-lang-java/openjdk-ev3/blob/78a9d35986ed51273d9a3bb9878cbfcbe2488bfb/upstream/softfloat.patch
> > > > > > > > > > > Regards,
> > > > > > > > > > > 
> > > > > > > > > > > Jakub
> > > > > > > > > > > 
> > > > > > > > > > > > /Erik
> > > > > > > > > > > 
> > > > > > > > > > > < raw patch at
> > > 
> > > 
https://raw.githubusercontent.com/ev3dev-lang-java/openjdk-ev3/78a9d35986ed51273d9a3bb9878cbfcbe2488bfb/upstream/softfloat.patch
> > > 