Re: RFR: 8352284: EXTRA_CFLAGS incorrectly applied to BUILD_LIBJVM src/hotspot C++ source files
On Wed, 19 Mar 2025 18:28:50 GMT, Erik Joelsson wrote: > The `EXTRA_[C|CXX]FLAGS` variables are not meant to be parameters to > SetupNativeCompilation Actually, the content of EXTRA_[C|CXX]FLAGS is hidden (and mixed together) inside the `JVM_CFLAGS` and get passed to as a parameter to `SetupNativeCompilation` eventually. The diff is whether we want to pass them implicitly (**base**) or explicitly (**patch**). The call sequence is: 1. Set up **JVM_CFLAGS** and append **EXTRA_CXXFLAGS** (make/autoconf/flags-cflags.m4#893) -> 2. Update **JVM_CFLAGS** and append **EXTRA_CFLAGS** (make/hotspot/lib/JvmFlags.gmk#L89) -> 3. Build libjvm -> 4. SetupJdkLibrary (pass **JVM_CFLAGS** as a parameter called **CFLAGS**, make/hotspot/lib/CompileJvm.gmk#L172) -> 5. SetupJdkNativeCompilation (make/common/JdkNativeCompilation.gmk#L477) -> 6. SetupNativeCompilation (make/common/JdkNativeCompilation.gmk#L456) -> 7. SetupCompilerFlags (set CXXFLAGS to **JVM_CFLAGS** now, make/common/NativeCompilation.gmk#L159, make/common/native/Flags.gmk#L131); CreateCompiledNativeFile (BASE_CXXFLAGS contains **JVM_CFLAGS** now, make/common/NativeCompilation.gmk#L179) -> 8. Compile C++ with BASE_CXXFLAGS that contains **JVM_CFLAGS** (make/common/native/CompileFile.gmk#L162) > Wouldn't just the change in JvmFlags.gmk be enough to solve your issue? No, it seems not doable. As mentioned above in the step 1 and 2, **JVM_CFLAGS** contains both EXTRA_CXXFLAGS and EXTRA_CFLAGS at that time, when calling SetupNativeCompilation, there is no information left about which flags were from EXTRA_[C|CXX]FLAGS and we are unable to do filter-out for C vs C++ files using corresponding flags. So, it is a dilemma, if we want to keep the parameters list of SetupNativeCompilation clean (untouched) we would have EXTRA_[C|CXX]FLAGS mixed together for compiling C and C++ files. Any other idea here please? - PR Comment: https://git.openjdk.org/jdk/pull/24115#issuecomment-2738738973
Re: RFR: 8352284: EXTRA_CFLAGS incorrectly applied to BUILD_LIBJVM src/hotspot C++ source files
On Thu, 20 Mar 2025 01:10:19 GMT, Patrick Zhang wrote: > Wouldn't just the change in JvmFlags.gmk be enough to solve your issue? Forgot to mention, one of my initial try-outs (hacking) was: Remove `$(EXTRA_CFLAGS)` from `JVM_CFLAGS`, based on an assumption that all source files under `$(TOPDIR)/src/hotspot` are (according to `hotspot/variant-server/libjvm/objs/BUILD_LIBJVM.d`) and will be C++ files only which has `$(EXTRA_CXXFLAGS)` and does **not** need `$(EXTRA_CFLAGS)` . This practically solved the issue I observed, however I don't think it was an elegant solution. Is this more acceptable than adding parameters to `SetupNativeCompilation` (actually the call to `SetupJdkLibrary)? diff --git a/make/hotspot/lib/JvmFlags.gmk b/make/hotspot/lib/JvmFlags.gmk index 97538da74c7..57b632ee532 100644 --- a/make/hotspot/lib/JvmFlags.gmk +++ b/make/hotspot/lib/JvmFlags.gmk @@ -91,7 +91,6 @@ JVM_CFLAGS += \ $(JVM_CFLAGS_TARGET_DEFINES) \ $(JVM_CFLAGS_FEATURES) \ $(JVM_CFLAGS_INCLUDES) \ -$(EXTRA_CFLAGS) \ # - PR Comment: https://git.openjdk.org/jdk/pull/24115#issuecomment-2739150006
RFR: 8352284: EXTRA_CFLAGS incorrectly applied to BUILD_LIBJVM src/hotspot C++ source files
Building jdk with `--with-extra-cflags='-Wno-incompatible-pointer-types'` triggers 1000+ warning messages like `cc1plus: warning: command-line option ‘-Wno-incompatible-pointer-types’ is valid for C/ObjC but not for C++`. The root cause is that `JVM_CFLAGS ` which contains both `EXTRA_CXXFLAGS` and `EXTRA_CFLAGS` when compiling `src/hotspot` C++ source files and building `BUILD_LIBJVM`. This PR does: 1. Not to append `EXTRA_CFLAGS` or `EXTRA_CXXFLAGS` into `JVM_CFLAGS` before calling `SetupJdkLibrary`, instead let `SetupCompilerFlags` accept and merge `EXTRA_CFLAGS` and `EXTRA_CXXFLAGS` passed from `SetupJdkLibrary` as parameters, so CPP compilation will only see `EXTRA_CXXFLAGS` as expected. 2. Correct `PCH_COMMAND` to use `EXTRA_CXXFLAGS` as precompiled.hpp.gch should not be compiled with `EXTRA_CFLAGS`. 3. Fixed `STATIC_LIB_CFLAGS` in `Flags.gmk` to `-DSTATIC_BUILD=1`, which was missed by [cbab40bc](https://github.com/openjdk/jdk/commit/cbab40bce45a2f58906be49c841178fa1dfd457e#diff-ab3ce05e795360030f19402fd0c2fad1dc1f7c5e7acc993cc4a2096cf31ccf40R114-R121) for the refactor of building static libs. Tests: Passed jdk building on an AArch64 Linux system and tier1 sanity tests, also passed OpenJDK GHA Sanity Checks. - Commit messages: - STATIC_LIB_CFLAGS in Flags.gmk should be updated to -DSTATIC_BUILD=1 too - precompiled.hpp.gch should be compiled with EXTRA_CXXFLAGS - 8352284: EXTRA_CFLAGS incorrectly applied to BUILD_LIBJVM src/hotspot C++ source files Changes: https://git.openjdk.org/jdk/pull/24115/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24115&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8352284 Stats: 8 lines in 5 files changed: 2 ins; 1 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/24115.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24115/head:pull/24115 PR: https://git.openjdk.org/jdk/pull/24115
Re: RFR: 8352044: Add --with-import-jvms to configure
On Tue, 18 Mar 2025 12:31:27 GMT, Magnus Ihse Bursie wrote: > And to be absolutely clear: this PR is just about adding new functionality > that was not present before. Sure but I think it's also an attractive nuisance. The VM is very tightly coupled to java.base and a few other core modules. I don't think the build should be supporting grabbing libjvm from another build or another location. Point taken that tools to build and test the JDK need to come from somewhere but it's not as tightly coupled as we have with the proposal here. For the folks targeting embedded/small environments then I think the right thing is to publish the packaged modules (JMOD files) for the target platform and then use `jlink` to create the run-image with the right VM + small set of modules that are appropriate for the target environment. - PR Comment: https://git.openjdk.org/jdk/pull/24063#issuecomment-2735829450
Re: RFR: 8352044: Add --with-import-jvms to configure
On Wed, 19 Mar 2025 09:11:05 GMT, Alan Bateman wrote: > For the folks targeting embedded/small environments then I think the right > thing is to publish the packaged modules (JMOD files) for the target platform > and then use `jlink` to create the run-image with the right VM + small set of > modules that are appropriate for the target environment. Just to make sure I'm understanding you correctly. Are you proposing that instead of building a single JDK distribution with multiple JVMs, they would build a separate JDK for each alternative JVM configuration and publish java.base.jmod from each of them for end users to pick from when generating their run-images? - PR Comment: https://git.openjdk.org/jdk/pull/24063#issuecomment-2736507794
