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

Severin Gehwolf sgehwolf at redhat.com
Fri Sep 7 16:56:42 UTC 2018


On Fri, 2018-09-07 at 09:12 -0700, Erik Joelsson wrote:
> On 2018-09-07 01:56, David Holmes wrote:
> > Hi Severin,
> > 
> > On 7/09/2018 6:01 PM, Severin Gehwolf wrote:
> > > Hi,
> > > 
> > > On Thu, 2018-09-06 at 10:55 -0700, Erik Joelsson wrote:
> > > > On 2018-09-06 10:29, Severin Gehwolf wrote:
> > > > > On Thu, 2018-09-06 at 09:55 -0700, Erik Joelsson wrote:
> > > > > Thanks, Erik. GCC supports -ffp-contract since 4.6. Clang has -ffp-
> > > > > contract too. Question is beginning from which version. That's why I'd
> > > > > expect for those flags to work on linux. Is there anything else I need
> > > > > to check?
> > > > > 
> > > > > Would it be preferred if I moved this into a block like this?
> > > > > 
> > > > > ifeq ($(TOOLCHAIN_TYPE), gcc)
> > > > >     [...]
> > > > > endif
> > > > 
> > > > Yes, making it conditional on toolchain type is what David was after.
> > > 
> > > Right. David, given that -ffp-contract is available for clang too, do
> > > you still want me to add this into a toolchain specific block?
> > 
> > Personally I'd prefer it but I will defer to Erik and Magnus on this.
> > 
> 
> 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?

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



More information about the core-libs-dev mailing list