Hello Severin,
On 09/12/2018 04:48 AM, Severin Gehwolf wrote:
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
I'm wondering why the possible issue of using -O3 on fdlibm would not affect
other parts of the JVM code (like the Opto code in libjvm.so) that afaics
are also built using -O3. Also, the gap of 1.05 for sin/cos, for instance,
still sounds like a regression to me.
Maybe a better approach to this would be to figure out a way to test -O3 and
-O2 with the real world case in which -O3 can have a bad impact. I'm not
sure if SPECjbb would qualify for that.
Best regards,
Gustavo
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/
