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

[Severin Gehwolf]

Hi,

On Thu, 2018-09-13 at 20:29 -0300, Gustavo Romero wrote:
> 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.

Fair enough. I'm not picky which of the two opt levels to go with as
long as we can agree on one. To me, both -O2 and -O3 seem reasonable
with -ffp-contract=off.

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

I'll let Andrew Haley comment on this one.

In my opinion, no *single* benchmark qualifies as "real-world-program"
as it would inadvertently look at one specific aspect. With hat being
said, any such benchmark would have to explicitly use StrictMath over
Math, no? Otherwise intrinsics would kick in. Is that likely? Also, as
Joe Darcy pointed out, some are now implemented in Java.

Cheers,
Severin

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