[PATCH v6] Add support for SoftFloat library on ARM

[Jakub Vaněk]

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_FEATURES
> > > > > +=
> > > > > -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
> > > > > > > >    >
> > > > > > > >