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





Reply via email to