Yes, this looks good, thanks for hanging in there!
/Erik
On 2018-09-14 02:04, Magnus Ihse Bursie wrote:
On 2018-09-14 10:37, Severin Gehwolf wrote:
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/
I think this looks reasonable, but let's hear what Erik has to say
about it also.
/Magnus
More thoughts?
Thanks,
Severin
