RFR: 8210416: [linux] Poor StrictMath performance due to non-optimized compilation

[Erik Joelsson]

On 2018-09-07 09:56, Severin Gehwolf wrote:
> On Fri, 2018-09-07 at 09:12 -0700, Erik Joelsson wrote:
>> To me it sounds like we want this flag if the toolchain is either gcc or
>> clang, so please make it conditional on that.
> Updated webrev:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/webrev.02/
>
> It now only sets FDLIBM_CFLAGS if gcc supports it and builds fdlibm
> without opt if not. Otherwise it uses -O3 and -ffp-contract=off. If the
> toolchain is clang and on linux it always uses -O3 and -ffp-
> contract=off. The reason, I've done this in configure is a potential
> follow-up fix for hotspot via JDK-8210425 where the CFLAGS could get
> re-used. Thoughts?
Instead of checking for compiler version, you could simply use the 
FLAGS_COMPILER_CHECK_ARGUMENTS macro. This would apply to both gcc and 
clang. We usually prefer this over version checks (unless a version is 
just known to be unstable with a certain flag, in which case a 
capability check will not work).

CoreLibrarkes.gmk:
43: I think "with" is superfluous

Otherwise I think this looks like the right direction.

/Erik
> Thanks,
> Severin
>
>> /Erik
>>> Thanks,
>>> David
>>>
>>>> Also note that solaris doesn't seem to be handling this code any
>>>> special, so there would be a difference between gcc + solaris and
>>>> solstudio + solaris. Rather hypothetical case, I suppose, but still ;-)
>>>> One of the arguments were that gcc is being used on other platforms
>>>> than linux.
>>>>
>>>>> You can also consider adding a capability check if you know that
>>>>> versions of the compiler that should still work don't have the feature.
>>>> One clarification: Does a capability check mean doing this at the
>>>> configure step or is there a way one has access to toolchain versions
>>>> in make files?
>>>>
>>>> Thanks,
>>>> Severin
>>>>
>>>>> /Erik
>>>>>> Thanks,
>>>>>> Severin
>>>>>>
>>>>>>> /Erik
>>>>>>>> Thanks,
>>>>>>>> Severin
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>> On 5/09/2018 11:12 PM, Severin Gehwolf wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Cross-posting this review-thread on core-libs-dev and build-dev as
>>>>>>>>>> this
>>>>>>>>>> is a build change, but affects fdlibm which is core-libs.
>>>>>>>>>>
>>>>>>>>>> With JDK-8170153 optimization for fdlibm code has been turned on
>>>>>>>>>> for
>>>>>>>>>> ppc64 s390 and aarch64. This patch intends to turn it on on all
>>>>>>>>>> arches
>>>>>>>>>> on Linux. I've not observed any precision issues. Is there a good
>>>>>>>>>> reason to not use -O3 -ffp-contract=off everywhere?
>>>>>>>>>>
>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210416
>>>>>>>>>> webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/webrev.01/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Testing: - java/lang/Math, java/lang/StrictMath tests (all pass).
>>>>>>>>>>               - Currently running through submit repo.
>>>>>>>>>>
>>>>>>>>>> A simple micro benchmark from JDK-8170153[1] shows these numbers
>>>>>>>>>> for
>>>>>>>>>> StrictMath:
>>>>>>>>>>
>>>>>>>>>> Function      | before     | after
>>>>>>>>>> ----------------------------------------------
>>>>>>>>>> sin           | 0m33.382s  | 0m18.731s
>>>>>>>>>> cos           | 0m31.562s  | 0m18.796s
>>>>>>>>>> tan           | 0m33.657s  | 0m21.093s
>>>>>>>>>> atan          | 0m5.714s   | 0m4.902s
>>>>>>>>>> log           | 0m6.212s   | 0m4.439s
>>>>>>>>>> log10         | 0m7.946s   | 0m5.543s
>>>>>>>>>> sqrt          | 0m0.481s   | 0m0.449s
>>>>>>>>>> cbrt          | 0m5.295s   | 0m5.214s
>>>>>>>>>> tanh          | 0m1.404s   | 0m1.307s
>>>>>>>>>> log1p         | 0m6.457s   | 0m5.131s
>>>>>>>>>> IEEEremainder | 0m10.629s  | 0m6.048s
>>>>>>>>>> atan2         | 0m8.037s   | 0m5.668s
>>>>>>>>>> hypot         | 0m2.171s   | 0m2.147s
>>>>>>>>>>
>>>>>>>>>> Thoughts?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Severin
>>>>>>>>>>
>>>>>>>>>>
>>>>>
>>