On Sat, 22 Mar 2025 10:09:46 GMT, Patrick Zhang <qpzh...@openjdk.org> wrote:
>> 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. > > Patrick Zhang has updated the pull request incrementally with three > additional commits since the last revision: > > - Fixed a typo > > Signed-off-by: Patrick Zhang <patr...@os.amperecomputing.com> > - Added comments to describe why EXTRA_CFLAGS is excluded from JVM_CFLAGS > > Signed-off-by: Patrick Zhang <patr...@os.amperecomputing.com> > - Revert the changes of adding new params to SetupNativeCompilation > > Signed-off-by: Patrick Zhang <patr...@os.amperecomputing.com> Hi @magicus , do you think my above clarifications regarding the unrelated code change and added comments acceptable, or do you still recommend removing those sections before merging? Thank you for confirming, and any advice on further changes. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24115#issuecomment-2766136731