[PATCH v6] Add support for SoftFloat library on ARM

Fri Dec 14 20:11:07 UTC 2018

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