On 2015-11-16 05:33, David Holmes wrote:
Hi Magnus,
I had a flick through most of the files. Overall seems okay but I
have a few queries below.
Replies inline.
On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote:
The build-infra project has collected a number of minor fixes and
changes during the new hotspot build development. It's a mix of
code
cleanup and new capabilities.
Not all of these new features are immediately beneficial to the JDK,
but
they will be needed for the upcoming new Hotspot build, and it will not
hurt to have them in mainline. (In fact, it will tremendously help
merging between mainline and build-infra.)
The fix addresses these issues:
In general:
* Break out hotspot configuration into hotspot.m4
* Long link lines uses @-files
* Consistently use -Wl instead of -Xlinker
* Improve clang on linux compilation
* Set shared library name explicitely on solaris
* Set correct shared library flag on Windows (-dll)
* Consistency fixes for build toolchain
* Bring compare script up to date
* General code/whitespace cleanup
* Additional functionality in MakeBase
In NativeCompilation.gmk:
* More efficient vardeps for per-file CFLAGS
* Fewer shell executions (means better performance on Windows)
* EXCLUDE_PATTERN and EXTRA_OBJECT_FILES
* Debug symbols on macosx (disabled for existing code to keep current
behavior)
Enabling debug info on macosx on existing jdk should be treated in a
follow-up bug.
Bug: https://bugs.openjdk.java.net/browse/JDK-8142907
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01
(It turned out that WebRev could not at the same time include files
from
multiple repos and track the history of a "hg cp":ied file. So I
created
an alternative revision here:
http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/
It does not include the jdk files, but hotspot.m4 might be easier to
understand)
flags.m4:
60 AC_SUBST(LEGACY_EXTRA_CFLAGS)
61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS)
62 AC_SUBST(LEGACY_EXTRA_LDFLAGS)
63
64 AC_SUBST(EXTRA_CFLAGS)
65 AC_SUBST(EXTRA_CXXFLAGS)
66 AC_SUBST(EXTRA_LDFLAGS)
IIRC we added the legacy flags purely to pass the cross-compilation
args through to hotspot. Not sure why we need both legacy and
non-legancy variants now ??
This is part of an ongoing effort to split CFLAGS_JDK into more
reasonable parts. The current model is that we throw all possible
flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et
al). While it was questionable even before, it did work when we only
built the JDK native libraries. Now that we need to build libjvm.so as
well, which has (for historical reason) a set of flags that are both
partially the same and partially different, this is not so good.
We would like to see a change to a situation where the different
"parts" that we build CFLAGS_JDK from would be handled separately, and
then combined as needed for JDK libraries, and for Hotspot. So,
currently we use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and then
drop them. In the new Hotspot build, we use the EXTRA_CFLAGS to add
them to the Hotspot flags. Of course, we could have used
LEGACY_EXTRA_CFLAGS, but we'd like to avoid using "LEGACY" for new stuff.
With that being said, I now realize that maybe this change will not be
needed anyway, at least not right now. Since doing a proper flag
cleanup is a major task, we decided to copy the CFLAGS_JDK behavior
for Hotspot flags as well in the current implementation of the new
hotspot build, and saving the cleanup for another day (so it will not
block the new build system). So I'll revert this part of the change
for now.
On Windows -LD is a superset of -dll, so it isn't obvious the change
is correct.
I hope Erik's reply to that was satisfactory.
---
jdk/make/lib/LibCommon.gmk
+ # Disable it here for the jdk binaries until we decide to enable them.
s/binaries/libraries/ ?
Sure, I'll fix.
Actually both this fragment and the one in
jdk/make/launcher/LauncherCommon.gmk I find confusing - what is the
relation with hotspot here, and the role of SetupNativeCompilation?
I hope Erik's reply to that is acceptable as well. As for the other
changes in the jdk repo, most of them is to either standardize on -Wl,
or to fix some places were we didn't do a proper $$ (instead of $) in
an macro that was supposed to be eval'd. (This was incorrect before as
well, but it broke the build when the LDFLAGS started having commas in
them.)
/Magnus
Thanks,
David
/Magnus
