8210425: [x86] sharedRuntimeTrig/sharedRuntimeTrans compiled without optimization

[Erik Joelsson]

On 2018-09-13 03:02, Severin Gehwolf wrote:
> Hi Erik,
>
> On Wed, 2018-09-12 at 10:02 -0700, Erik Joelsson wrote:
>> Hello Severin,
>>
>> In configure, we now set FDLIBM_CFLAGS based on toolchain type and
>> capabilities. In JvmOverrideFiles.gmk, the use of it is still
>> conditional on Linux. I don't think it should be. We already have the
>> conditionals we need.
> Thanks for the review. Updated webrev:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210425/webrev.03/
>
> I wasn't sure whether the precompiled headers handling for gcc/clang is
> right and was reluctant to move it out of the linux conditional. The
> assumption on my end is that if headers are compiled with -O3, we can't
> used them for any other opt level. It should still all work. Thoughts?
Looking at this again, I now realize the whole file has the treatment 
for these files repeated for each OS, with slight variations. This 
becomes a clash with the change we are now attempting which is toolchain 
oriented. I think the easiest way here would be to keep it OS separated 
for now by reverting to your previous patch.

You could consider putting the FDLIBM_CFLAGS conditional block outside 
the linux block however and apply the "$(FDLIBM_CFLAGS) 
$(LIBJVM_FDLIBM_COPY_OPT_FLAG)" flags on the lines in the macosx block 
further down as well. The changes for fdlibm as they are now will apply 
on macosx since we use clang there, so the jvm change should too.

/Erik
> Thanks,
> Severin
>
>> On 2018-09-12 05:44, Severin Gehwolf wrote:
>>> Hi,
>>>
>>> Updated webrev since fdlibm build change seems to have settled:
>>> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210425/webrev.02/
>>>
>>> Optimization is being done at level -O2 (CXX_O_FLAG_NORM) with
>>> -ffp-contract=off when the toolchain (gcc/clang at this point)
>>> supports it. Otherwise no optimization will be done. Those overrides
>>> are no longer in an x86 specific block. Thus, ppc64, aarch64, s390x
>>> will get correct settings too.
>>>
>>> How does this look?
>>>
>>> Thanks,
>>> Severin
>>>
>>> On Wed, 2018-09-12 at 07:16 +0000, Schmidt, Lutz wrote:
>>>> I totally agree with Andrew's statement. FP calculations should be
>>>> evaluated as the programmer wrote them down. All fiddling around with
>>>> sequence or rounding is evil. You lose reproducibility of results.
>>>> Regards,
>>>> Lutz
>>>>
>>>> On 08.09.18, 11:26, "hotspot-dev on behalf of Andrew Haley" <hotspot-dev-bounces at openjdk.java.net on behalf of aph at redhat.com> wrote:
>>>>
>>>>       On 09/06/2018 03:32 PM, Severin Gehwolf wrote:
>>>>       > Right. I should note that ppc64, s390x and aarch64 ports don't have
>>>>       > this optimization turned off as those overrides are in a x86 specific
>>>>       > block. It appears it hasn't caused issues for these ports so far.
>>>>       
>>>>       That's just dumb luck. We really should turn off the merging of FP
>>>>       operations on all platforms.
>>>>       
>>>>       --
>>>>       Andrew Haley
>>>>       Java Platform Lead Engineer
>>>>       Red Hat UK Ltd. <https://www.redhat.com>
>>>>       EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>>>>       
>>>>
>>