Hi David, On Wed, 2018-09-12 at 13:57 +1000, David Holmes wrote: > Hi Severin, > > Sorry I'm a bit confused now. > > Originally for ppc/s390x/aarch64 the optimization level for fdlibm was > HIGH. But now it will be LOW due to: > > 45 ifneq ($(FDLIBM_CFLAGS), ) > 46 BUILD_LIBFDLIBM_OPTIMIZATION := LOW > > as those platforms will use gcc/clang with support for -ffp-contract and > so FDLIBM_CFLAGS will not be empty. > > ??
Correct. That was done based on Andrew Haley's comment here: http://mail.openjdk.java.net/pipermail/build-dev/2018-September/023160.html I've done some measurements with -O2 and -O3. -O2 is sufficient for a 2.75 speedup for sin/cos on ppc64le as compared to the -O0 baseline. On the other hand the speedup of -O3 as compared to -O2 is only 1.05 for sin/cos on ppc64le. Since the difference between them were not huge, I've used -O2, a.k.a LOW. See also: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/microbenchmarks_results/ To me it seems -O2 is sufficiently good. Note that HIGH == -O3 and LOW == -O2 on gcc arches. Does that clear things up? FWIW, I'm happy to change it back to HIGH if there are concerns. See also Gustavo's reply here for some more data points: http://mail.openjdk.java.net/pipermail/build-dev/2018-September/023193.html Thanks, Severin > David > > On 12/09/2018 2:14 AM, Severin Gehwolf wrote: > > Hi Erik, > > > > Thanks for the review! > > > > On Tue, 2018-09-11 at 08:57 -0700, Erik Joelsson wrote: > > > Hello Severin, > > > > > > Even if using the macro, I still think you need to add a condition on > > > the compiler types where the switch can be reasonably expected to exist. > > > > How about this? > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/webrev.05/ > > > > Thanks, > > Severin > > > > > On 2018-09-11 05:02, Severin Gehwolf wrote: > > > > On Mon, 2018-09-10 at 09:29 -0700, Erik Joelsson wrote: > > > > > I see. I was not aware of that issue, but we clearly need to file a > > > > > bug > > > > > for it and fix it. In this case I think it's fine to us the macro > > > > > however. > > > > > > > > OK. Update webrev, which now uses FLAGS_COMPILER_CHECK_ARGUMENTS. > > > > > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/webrev.04/ > > > > > > > > Micro-benchmark results from running [1] for x86_64 and ppc64le are > > > > here (-O2 is sufficient it seems): > > > > > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/microbenchmarks_results/ > > > > > > > > More thoughts? > > > > > > > > Thanks, > > > > Severin > > > > > > > > [1] https://github.com/gromero/strictmath/ > > > > > > > > > >