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
