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.
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 actually looks weird to me too. I can find no usage of EXTRA_CFLAGS
without the LEGACY_ prefix.
On Windows -LD is a superset of -dll, so it isn't obvious the change
is correct.
It seems, /LD is a compiler option and /DLL is a linker option. Setting
/LD to cl.exe makes it set /DLL to link.exe. Since we link using
link.exe we must use -dll. Looking closer at the change, we are actually
setting SHARED_LIBRARY_FLAGS=-DL on windows, but then never using that
variable. Instead hard coding -dll further down. This change corrects
SHARED_LIBRARY_FLAGS to -dll and then uses that value.
---
jdk/make/lib/LibCommon.gmk
+ # Disable it here for the jdk binaries until we decide to enable them.
s/binaries/libraries/ ?
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?
In SetupNativeCompilation, we currently disable debug symbols creation
on Macosx. Back in JDK 8 when we converted the jdk build to build-infra,
that was how the old build worked and we just mimicked. Until now, we
haven't gotten around to fixing debug symbols support for macosx. In the
build-infra forest, since debug symbols are needed for hotspot, we have
implemented support in SetupNativeCompilation. Magnus intends to bring
this support into JDK 9 with this change, the motivation being to reduce
differences between build-infra and JDK 9 which are complicating
merging. We think actually enabling debug symbols support for the JDK
libraries should rather be a separate change, that we want to follow up
with soon after.
/Erik
Thanks,
David
/Magnus
