8210425: [x86] sharedRuntimeTrig/sharedRuntimeTrans compiled without optimization

[Severin Gehwolf]

On Thu, 2018-09-13 at 10:39 -0700, Erik Joelsson wrote:
> On 2018-09-13 03:02, Severin Gehwolf wrote:
> > Hi Erik,
> > 
> > On Wed, 2018-09-12 at 10:02 -0700, Erik Joelsson wrote:
> > > Hello Severin,
> > > 
> > > In configure, we now set FDLIBM_CFLAGS based on toolchain type and
> > > capabilities. In JvmOverrideFiles.gmk, the use of it is still
> > > conditional on Linux. I don't think it should be. We already have the
> > > conditionals we need.
> > 
> > Thanks for the review. Updated webrev:
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210425/webrev.03/
> > 
> > I wasn't sure whether the precompiled headers handling for gcc/clang is
> > right and was reluctant to move it out of the linux conditional. The
> > assumption on my end is that if headers are compiled with -O3, we can't
> > used them for any other opt level. It should still all work. Thoughts?
> 
> Looking at this again, I now realize the whole file has the treatment 
> for these files repeated for each OS, with slight variations. This 
> becomes a clash with the change we are now attempting which is toolchain 
> oriented. I think the easiest way here would be to keep it OS separated 
> for now by reverting to your previous patch.
> 
> You could consider putting the FDLIBM_CFLAGS conditional block outside 
> the linux block however and apply the "$(FDLIBM_CFLAGS) 
> $(LIBJVM_FDLIBM_COPY_OPT_FLAG)" flags on the lines in the macosx block 
> further down as well. The changes for fdlibm as they are now will apply 
> on macosx since we use clang there, so the jvm change should too.

OK, done:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210425/webrev.04/

More thoughts?

Thanks,
Severin