Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support
On Wed, 16 Sep 2020 20:26:10 GMT, Monica Beckwith wrote: > This is a continuation of > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html > > Changes since then: > * We've improved the write barrier as suggested by Andrew [1] > * The define-guards around R18 have been changed to `R18_RESERVED`. This will > be enabled for Windows only for now but > will be required for the upcoming macOS+Aarch64 [2] port as well. > * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov > in our PR for now and built the > Windows-specific CPU feature detection on top of it. > > [1] > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html > [2] https://openjdk.java.net/jeps/8251280 make/autoconf/toolchain.m4 line 902: > 900: BUILD_DEVKIT_TOOLCHAIN_PATH="$BUILD_DEVKIT_ROOT/bin" > 901: fi > 902: UTIL_PREPEND_TO_PATH([PATH],$BUILD_DEVKIT_TOOLCHAIN_PATH) Here is a problem. In our linux cross compile build, we rely on the PATH being completely overwritten with the paths from the devkit here. Otherwise the UTIL_REQUIRE_PROGS may find /usr/bin/cc before $BUILD_DEVKIT_TOOLCHAIN_PATH/gcc. This is the reason my linux-aarch64 (cross compile) build failed. The system installed cc was too old to recognize the -stdc=c++14 argument. - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support
On Fri, 18 Sep 2020 20:32:36 GMT, Erik Joelsson wrote: >> This is a continuation of >> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html >> >> Changes since then: >> * We've improved the write barrier as suggested by Andrew [1] >> * The define-guards around R18 have been changed to `R18_RESERVED`. This >> will be enabled for Windows only for now but >> will be required for the upcoming macOS+Aarch64 [2] port as well. >> * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov >> in our PR for now and built the >> Windows-specific CPU feature detection on top of it. >> >> [1] >> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html >> [2] https://openjdk.java.net/jeps/8251280 > > make/autoconf/toolchain.m4 line 902: > >> 900: BUILD_DEVKIT_TOOLCHAIN_PATH="$BUILD_DEVKIT_ROOT/bin" >> 901: fi >> 902: UTIL_PREPEND_TO_PATH([PATH],$BUILD_DEVKIT_TOOLCHAIN_PATH) > > Here is a problem. In our linux cross compile build, we rely on the PATH > being completely overwritten with the paths > from the devkit here. Otherwise the UTIL_REQUIRE_PROGS may find /usr/bin/cc > before $BUILD_DEVKIT_TOOLCHAIN_PATH/gcc. > This is the reason my linux-aarch64 (cross compile) build failed. The system > installed cc was too old to recognize > the -stdc=c++14 argument. I assume you need the rest of the PATH on Windows. - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support
On Fri, 18 Sep 2020 15:34:26 GMT, Erik Joelsson wrote: >> Build changes look good to me. I will take this branch for a spin. > > Our linux-aarch64 build fails with this: > cc: error: unrecognized command line option '-std=c++14' > when compiling > build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch > > I'm trying to configure a windows-aarch64 build, but it fails on fixpath. Is > this something you are also experiencing, > and if so, how are you addressing it? Hey @erikj79, thank you so much for giving it a try! > Our linux-aarch64 build fails with this: > cc: error: unrecognized command line option '-std=c++14' > when compiling > build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch Hmm, that's interesting. What environment is that exactly? What `configure` line are you using there? We have tested on such a system: $ cat /etc/issue Ubuntu 19.10 \n \l $ bash configure --with-boot-jdk=/home/beurba/work/jdk-16+13 --with-jtreg $ make clean CONF=linux-aarch64-server-release $ make images JOBS=255 LOG=info CONF=linux-aarch64-server-release $ ./build/linux-aarch64-server-release/images/jdk/bin/java -XshowSettings:properties -version 2>&1 | grep aarch64 java.home = /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk os.arch = aarch64 sun.boot.library.path = /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk/lib > I'm trying to configure a windows-aarch64 build, but it fails on fixpath. Is > this something you are also experiencing, > and if so, how are you addressing it? Yes. As far as I understand, the problem is that `fixpath.exe` isn't built properly when doing cross-compiling on Windows targets (as it hasn't been a thing so far). We use a workaround internally https://gist.github.com/lewurm/c099a4b5fcd8a182510cbdeebcb41f77 , but a proper solution is under discussion on build-dev: https://mail.openjdk.java.net/pipermail/build-dev/2020-July/027872.html - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support
On Fri, 18 Sep 2020 13:33:07 GMT, Erik Joelsson wrote: >> This is a continuation of >> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html >> >> Changes since then: >> * We've improved the write barrier as suggested by Andrew [1] >> * The define-guards around R18 have been changed to `R18_RESERVED`. This >> will be enabled for Windows only for now but >> will be required for the upcoming macOS+Aarch64 [2] port as well. >> * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov >> in our PR for now and built the >> Windows-specific CPU feature detection on top of it. >> >> [1] >> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html >> [2] https://openjdk.java.net/jeps/8251280 > > Build changes look good to me. I will take this branch for a spin. Our linux-aarch64 build fails with this: cc: error: unrecognized command line option '-std=c++14' when compiling build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch I'm trying to configure a windows-aarch64 build, but it fails on fixpath. Is this something you are also experiencing, and if so, how are you addressing it? - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: JDK-8253239: Disable VS warning C4307 [v3]
On Fri, 18 Sep 2020 13:58:57 GMT, Matthias Baesken wrote: >> hello, this fixes the build when using VS2017. VS2019 does not have the >> issue as far as I know. >> failure was >> >>> ./test/hotspot/gtest/utilities/test_align.cpp(96): error C2220: warning >>> treated as error - no 'object' file generated >>> ./test/hotspot/gtest/utilities/test_align.cpp(156): note: see reference to >>> function template instantiation 'void >>> static_test_alignments(void)' being compiled with >>> [ >>> T=int64_t, >>> A=uint8_t >>> ] >>> ./test/hotspot/gtest/utilities/test_align.cpp(162): note: see reference to >>> function template instantiation 'void >>> test_alignments(void)' being compiled >>> ./test/hotspot/gtest/utilities/test_align.cpp(96): warning >>> C4307: '+': integral constant overflow >>> >>> >> >> This might be related to an issue fixed at least in some versions of VS2019 >> , that is discussed here : >> >> https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html >> >> https://bugs.openjdk.java.net/browse/JDK-8253239 > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > JDK-8253239: Disable VS warning C4307 > > Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> Marked as reviewed by mdoerr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/237
Integrated: JDK-8253239: Disable VS warning C4307
On Fri, 18 Sep 2020 08:22:56 GMT, Matthias Baesken wrote: > hello, this fixes the build when using VS2017. VS2019 does not have the issue > as far as I know. > failure was > >> ./test/hotspot/gtest/utilities/test_align.cpp(96): error C2220: warning >> treated as error - no 'object' file generated >> ./test/hotspot/gtest/utilities/test_align.cpp(156): note: see reference to >> function template instantiation 'void >> static_test_alignments(void)' being compiled with >> [ >> T=int64_t, >> A=uint8_t >> ] >> ./test/hotspot/gtest/utilities/test_align.cpp(162): note: see reference to >> function template instantiation 'void >> test_alignments(void)' being compiled >> ./test/hotspot/gtest/utilities/test_align.cpp(96): warning >> C4307: '+': integral constant overflow >> >> > > This might be related to an issue fixed at least in some versions of VS2019 , > that is discussed here : > > https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html > > https://bugs.openjdk.java.net/browse/JDK-8253239 This pull request has now been integrated. Changeset: 52c28b86 Author:Matthias Baesken URL: https://git.openjdk.java.net/jdk/commit/52c28b86 Stats: 4 lines in 1 file changed: 0 ins; 4 del; 0 mod 8253239: Disable VS warning C4307 Reviewed-by: mdoerr, erikj - PR: https://git.openjdk.java.net/jdk/pull/237
Re: RFR: JDK-8253239: Disable VS warning C4307 [v3]
On Fri, 18 Sep 2020 13:58:57 GMT, Matthias Baesken wrote: >> hello, this fixes the build when using VS2017. VS2019 does not have the >> issue as far as I know. >> failure was >> >>> ./test/hotspot/gtest/utilities/test_align.cpp(96): error C2220: warning >>> treated as error - no 'object' file generated >>> ./test/hotspot/gtest/utilities/test_align.cpp(156): note: see reference to >>> function template instantiation 'void >>> static_test_alignments(void)' being compiled with >>> [ >>> T=int64_t, >>> A=uint8_t >>> ] >>> ./test/hotspot/gtest/utilities/test_align.cpp(162): note: see reference to >>> function template instantiation 'void >>> test_alignments(void)' being compiled >>> ./test/hotspot/gtest/utilities/test_align.cpp(96): warning >>> C4307: '+': integral constant overflow >>> >>> >> >> This might be related to an issue fixed at least in some versions of VS2019 >> , that is discussed here : >> >> https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html >> >> https://bugs.openjdk.java.net/browse/JDK-8253239 > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > JDK-8253239: Disable VS warning C4307 > > Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/237
Re: RFR: JDK-8253239: Disable VS warning C4307 [v3]
> hello, this fixes the build when using VS2017. VS2019 does not have the issue > as far as I know. > failure was > >> ./test/hotspot/gtest/utilities/test_align.cpp(96): error C2220: warning >> treated as error - no 'object' file generated >> ./test/hotspot/gtest/utilities/test_align.cpp(156): note: see reference to >> function template instantiation 'void >> static_test_alignments(void)' being compiled with >> [ >> T=int64_t, >> A=uint8_t >> ] >> ./test/hotspot/gtest/utilities/test_align.cpp(162): note: see reference to >> function template instantiation 'void >> test_alignments(void)' being compiled >> ./test/hotspot/gtest/utilities/test_align.cpp(96): warning >> C4307: '+': integral constant overflow >> >> > > This might be related to an issue fixed at least in some versions of VS2019 , > that is discussed here : > > https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html > > https://bugs.openjdk.java.net/browse/JDK-8253239 Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: JDK-8253239: Disable VS warning C4307 Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - Changes: - all: https://git.openjdk.java.net/jdk/pull/237/files - new: https://git.openjdk.java.net/jdk/pull/237/files/4e82edcb..af48bb31 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=237=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=237=01-02 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/237.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/237/head:pull/237 PR: https://git.openjdk.java.net/jdk/pull/237
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support
On Wed, 16 Sep 2020 20:26:10 GMT, Monica Beckwith wrote: > This is a continuation of > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html > > Changes since then: > * We've improved the write barrier as suggested by Andrew [1] > * The define-guards around R18 have been changed to `R18_RESERVED`. This will > be enabled for Windows only for now but > will be required for the upcoming macOS+Aarch64 [2] port as well. > * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov > in our PR for now and built the > Windows-specific CPU feature detection on top of it. > > [1] > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html > [2] https://openjdk.java.net/jeps/8251280 Build changes look good to me. I will take this branch for a spin. - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: JDK-8253239: Disable VS warning C4307 [v2]
On Fri, 18 Sep 2020 10:00:55 GMT, Aleksey Shipilev wrote: >> Matthias Baesken has refreshed the contents of this pull request, and >> previous commits have been removed. The >> incremental views will show differences compared to the previous content of >> the PR. The pull request contains one new >> commit since the last revision: >> JDK-8253239 > > This looks fine to me, but build maintainers should approve. I left a suggestion for a comment. You should be able to just accept it if you are happy with it. If you would rather write something yourself, I recommend committing without --amend. The Skara bot will automatically squash all your changes before merging this to the main repo. - PR: https://git.openjdk.java.net/jdk/pull/237
Re: RFR: JDK-8253239: Disable VS warning C4307 [v2]
On Fri, 18 Sep 2020 10:05:10 GMT, Matthias Baesken wrote: >> hello, this fixes the build when using VS2017. VS2019 does not have the >> issue as far as I know. >> failure was >> >>> ./test/hotspot/gtest/utilities/test_align.cpp(96): error C2220: warning >>> treated as error - no 'object' file generated >>> ./test/hotspot/gtest/utilities/test_align.cpp(156): note: see reference to >>> function template instantiation 'void >>> static_test_alignments(void)' being compiled with >>> [ >>> T=int64_t, >>> A=uint8_t >>> ] >>> ./test/hotspot/gtest/utilities/test_align.cpp(162): note: see reference to >>> function template instantiation 'void >>> test_alignments(void)' being compiled >>> ./test/hotspot/gtest/utilities/test_align.cpp(96): warning >>> C4307: '+': integral constant overflow >>> >>> >> >> This might be related to an issue fixed at least in some versions of VS2019 >> , that is discussed here : >> >> https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html >> >> https://bugs.openjdk.java.net/browse/JDK-8253239 > > Matthias Baesken has refreshed the contents of this pull request, and > previous commits have been removed. The > incremental views will show differences compared to the previous content of > the PR. The pull request contains one new > commit since the last revision: > JDK-8253239 make/autoconf/flags-cflags.m4 line 137: > 135: WARNINGS_ENABLE_ALL="-W3" > 136: DISABLED_WARNINGS="4800" > 137: if test "x$TOOLCHAIN_VERSION" = x2017; then I would like a comment here explaining why we need to disable this warning, since it's caused by a faulty behavior in the compiler. make/autoconf/flags-cflags.m4 line 138: > 136: DISABLED_WARNINGS="4800" > 137: if test "x$TOOLCHAIN_VERSION" = x2017; then > 138: DISABLED_WARNINGS+=" 4307" Suggestion: # VS2017 incorrectly triggers this warning for constexpr DISABLED_WARNINGS+=" 4307" - PR: https://git.openjdk.java.net/jdk/pull/237
Re: RFR: 8252998: ModuleWrapper.gmk doesn't consult include path
On Fri, 18 Sep 2020 12:07:45 GMT, Adam Farley wrote: > A change made to ModuleWrapper.gmk as part of JDK-8244044 means that an > included makefile is found in the current > directory, so Make doesn't check the include dirs for overriding gmk files. > Recommend a minor, partial reversion of this changeset to ensure any override > files are included instead. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8252998 Looks good. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/250
Re: RFR: 8253000: Remove redundant MAKE_SUBDIR argument
On Fri, 18 Sep 2020 11:37:06 GMT, Adam Farley wrote: > As part of JDK-8244044, MAKE_SUBDIR is no longer used in > DeclareRecipesForPhase inside MakeSupport.gmk. > > Therefore, it seems sensible to modify the instances where this is passed to > DeclareRecipesForPhase, and to remove > references to it from the associated MakeSupport.gmk comment. > Bug: https://bugs.openjdk.java.net/browse/JDK-8253000 Looks good, thanks for cleaning this up! - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/249
RFR: 8252998: ModuleWrapper.gmk doesn't consult include path
A change made to ModuleWrapper.gmk as part of JDK-8244044 means that an included makefile is found in the current directory, so Make doesn't check the include dirs for overriding gmk files. Recommend a minor, partial reversion of this changeset to ensure any override files are included instead. Bug: https://bugs.openjdk.java.net/browse/JDK-8252998 - Commit messages: - 8252998: ModuleWrapper.gmk doesn't consult include path Changes: https://git.openjdk.java.net/jdk/pull/250/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=250=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252998 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/250.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/250/head:pull/250 PR: https://git.openjdk.java.net/jdk/pull/250
RFR: 8253000: Remove redundant MAKE_SUBDIR argument
As part of JDK-8244044, MAKE_SUBDIR is no longer used in DeclareRecipesForPhase inside MakeSupport.gmk. Therefore, it seems sensible to modify the instances where this is passed to DeclareRecipesForPhase, and to remove references to it from the associated MakeSupport.gmk comment. Bug: https://bugs.openjdk.java.net/browse/JDK-8253000 - Commit messages: - 8253000: Remove redundant MAKE_SUBDIR argument Changes: https://git.openjdk.java.net/jdk/pull/249/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=249=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253000 Stats: 7 lines in 2 files changed: 0 ins; 7 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/249.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/249/head:pull/249 PR: https://git.openjdk.java.net/jdk/pull/249
Re: RFR: 8252999: Cleanup: replace .equals("") with .isEmpty() within all codebase
On Fri, 18 Sep 2020 02:48:15 GMT, Stuart Marks wrote: >> Our local Santuario maintainer says: >> >> In general, changes to Apache Santuario should also be made at Apache so we >> stay in sync. > > Hi @doom369, I hope we didn't end up wasting too much of your time with this. > I wanted to respond to a comment you made > earlier in this PR, >> I have in mind dozens of improvements all over the code like this one. > > It's hard to see, but as you discovered, the JDK has different groups of > people maintaining different areas, and > sometimes there are hidden constraints on those different areas, for example, > to avoid divergence with upstream source > bases. And as you discovered, sometimes those source bases might need to > maintain compatibility with an older JDK ... > so we don't want to update this code even though it's IN the JDK. Those kind > of constraints generally don't apply to > code in the java.base module, since there's nothing upstream of it, and by > definition it cannot depend on anything > outside the JDK. Generally -- though there are exceptions -- we're more > receptive to keeping stuff in java.base (and > sometimes related modules close to the core) on the latest and greatest > stuff. There are some constraints, however. > There are some things we can't use too early during initialization of the > JDK, such as lambdas. Also, there is some > code lurking around that is sometimes executed by the boot JDK, which is > typically one release behind. (This is > definitely the case for tools like javac, but I think it might also apply to > some things in base.) Anyway, if you'd > like to pursue some of these improvements, drop a note to > core-libs-dev@openjdk and we can talk about it. Thanks. @stuart-marks thanks. Sure, I understand that maintaining OpenJDK is not a simple task. I thought as change is super simple without any side effects it can go through. But I was wrong. That's fine. I'll focus on `java.base` when I have some free time. - PR: https://git.openjdk.java.net/jdk/pull/29
Re: RFR: JDK-8247589: Implementation of Alpine Linux/x64 Port [v2]
On Mon, 14 Sep 2020 06:30:50 GMT, Aleksei Voitylov wrote: >> Marked as reviewed by dholmes (Reviewer). > > thank you Alan, Erik, and David! When the JEP becomes Targeted, I'll use this > PR to integrate the changes. I added the contributors that could be found in the portola project commits. If anyone knows some other contributors I missed, I'll be happy to stand corrected. - PR: https://git.openjdk.java.net/jdk/pull/49
RFR: 8248238: Implementation of JEP: Windows AArch64 Support
This is a continuation of https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html Changes since then: * We've improved the write barrier as suggested by Andrew [1] * The define-guards around R18 have been changed to `R18_RESERVED`. This will be enabled for Windows only for now but will be required for the upcoming macOS+Aarch64 [2] port as well. * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov in our PR for now and built the Windows-specific CPU feature detection on top of it. [1] https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html [2] https://openjdk.java.net/jeps/8251280 - Commit messages: - 8248787: G1: Workaround MSVC bug - 8248670: Windows: Exception handling support on AArch64 - 8248660: AArch64: Make _clear_cache and _nop portable - 8248659: AArch64: Extend CPU Feature detection - 8248656: Add Windows AArch64 platform support code - 8248498: Add build system support for Windows AArch64 - 8248681: AArch64: MSVC doesn't support __PRETTY_FUNCTION__ - 8248663: AArch64: Avoid existing macros/keywords of MSVC - 8248676: AArch64: Add workaround for LITable constructor - 8248500: AArch64: Remove the r18 dependency on Windows AArch64 (regenerate tests) - ... and 3 more: https://git.openjdk.java.net/jdk/compare/68da63dc...26e4af3a Changes: https://git.openjdk.java.net/jdk/pull/212/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=212=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8248238 Stats: 4230 lines in 69 files changed: 2406 ins; 273 del; 1551 mod Patch: https://git.openjdk.java.net/jdk/pull/212.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/212/head:pull/212 PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: JDK-8253239: Disable VS warning C4307 [v2]
On Fri, 18 Sep 2020 08:57:36 GMT, Aleksey Shipilev wrote: >> Matthias Baesken has refreshed the contents of this pull request, and >> previous commits have been removed. The >> incremental views will show differences compared to the previous content of >> the PR. The pull request contains one new >> commit since the last revision: >> JDK-8253239 > > make/autoconf/flags-cflags.m4 line 138: > >> 136: DISABLED_WARNINGS="4800" >> 137: if test "x$TOOLCHAIN_VERSION" = x2017; then >> 138: DISABLED_WARNINGS+=" 4307" > > Seems like this file uses 2 spaces as indenting, 4 spaces are used here. You Are right, I adjusted the indenting to 2 sapces and did a git commit --amend and repushed it. - PR: https://git.openjdk.java.net/jdk/pull/237
Re: RFR: JDK-8253239: Disable VS warning C4307 [v2]
> hello, this fixes the build when using VS2017. VS2019 does not have the issue > as far as I know. > failure was > >> ./test/hotspot/gtest/utilities/test_align.cpp(96): error C2220: warning >> treated as error - no 'object' file generated >> ./test/hotspot/gtest/utilities/test_align.cpp(156): note: see reference to >> function template instantiation 'void >> static_test_alignments(void)' being compiled with >> [ >> T=int64_t, >> A=uint8_t >> ] >> ./test/hotspot/gtest/utilities/test_align.cpp(162): note: see reference to >> function template instantiation 'void >> test_alignments(void)' being compiled >> ./test/hotspot/gtest/utilities/test_align.cpp(96): warning >> C4307: '+': integral constant overflow >> >> > > This might be related to an issue fixed at least in some versions of VS2019 , > that is discussed here : > > https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html > > https://bugs.openjdk.java.net/browse/JDK-8253239 Matthias Baesken has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: JDK-8253239 - Changes: - all: https://git.openjdk.java.net/jdk/pull/237/files - new: https://git.openjdk.java.net/jdk/pull/237/files/0d1d38b3..4e82edcb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=237=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=237=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/237.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/237/head:pull/237 PR: https://git.openjdk.java.net/jdk/pull/237
Re: RFR: JDK-8253239: Disable VS warning C4307 [v2]
On Fri, 18 Sep 2020 10:00:55 GMT, Matthias Baesken wrote: >> hello, this fixes the build when using VS2017. VS2019 does not have the >> issue as far as I know. >> failure was >> >>> ./test/hotspot/gtest/utilities/test_align.cpp(96): error C2220: warning >>> treated as error - no 'object' file generated >>> ./test/hotspot/gtest/utilities/test_align.cpp(156): note: see reference to >>> function template instantiation 'void >>> static_test_alignments(void)' being compiled with >>> [ >>> T=int64_t, >>> A=uint8_t >>> ] >>> ./test/hotspot/gtest/utilities/test_align.cpp(162): note: see reference to >>> function template instantiation 'void >>> test_alignments(void)' being compiled >>> ./test/hotspot/gtest/utilities/test_align.cpp(96): warning >>> C4307: '+': integral constant overflow >>> >>> >> >> This might be related to an issue fixed at least in some versions of VS2019 >> , that is discussed here : >> >> https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html >> >> https://bugs.openjdk.java.net/browse/JDK-8253239 > > Matthias Baesken has refreshed the contents of this pull request, and > previous commits have been removed. The > incremental views will show differences compared to the previous content of > the PR. The pull request contains one new > commit since the last revision: > JDK-8253239 This looks fine to me, but build maintainers should approve. - PR: https://git.openjdk.java.net/jdk/pull/237
Re: [8u] RFR: 8252975: [8u] JDK-8252395 breaks the build for --with-native-debug-symbols=internal
Hi Andrew,
On Fri, 2020-09-18 at 06:40 +0100, Andrew Hughes wrote:
> On 20:16 Wed 09 Sep , Severin Gehwolf wrote:
> > Hi,
> >
> > Please review this 8u (jdk8u/jdk8u-dev tree) fix for JDK-8252395 that
> > I've pushed today. Thanks for Zhengyu Gu for noticing it. The pushed
> > fix added the java.debuginfo and unpack.debuginfo make targets on the
> > condition of ENABLE_DEBUG_SYMBOLS=true, which is insufficient. It needs
> > another check on POST_STRIP_CMD which is set to non-empty for --with-
> > native-debug-symbols={external,zipped}, and is indeed empty for --with-
> > native-debug-symbols=internal. For the --with-native-debug-symbols=none
> > case we have ENABLE_DEBUG_SYMBOLS=false.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8252975
> > webrev: https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8252975/01/webrev/
> >
> > Testing: Builds with
> > --with-native-debug-symbols={none,internal,external,zipped} on Linux x86_64
> >
> > OK?
> >
> > Thanks,
> > Severin
> >
>
> Build is still broken for me with this patch:
>
> /usr/bin/cp /home/ahughes/builder/8u-dev/jdk/objs/java_objs/java.diz
> /home/ahughes/builder/8u-dev/jdk/bin/java.diz
> /usr/bin/cp: cannot stat
> '/home/ahughes/builder/8u-dev/jdk/objs/java_objs/java.diz': No such file or
> directory
>
> where --with-native-debug-symbols is not set (pre-JDK-8207324)
Hmm, I'm not sure how you are building. I've checked builds with:
$ bash configure \
--with-boot-jdk="/some/boot/jdk" \
--with-extra-cflags=-Wno-error \
--disable-zip-debug-info
$ make images
and
$ bash configure \
--with-boot-jdk="/some/boot/jdk" \
--with-extra-cflags=-Wno-error
$ make images
and
$ bash configure \
--with-boot-jdk="/some/boot/jdk" \
--with-extra-cflags=-Wno-error \
--disable-debug-symbols
$ make images
All seem to work fine for me. Are you sure you are not
setting POST_STRIP_CMD or the like explicitly somewhere else. Please
post the exact configure/make invocations you are using.
FWIW, that bug you've referenced seems wrong:
8207324: aarch64 jtreg: assert in TestOptionsWithRanges.jtr
You probably meant JDK-8036003:
8036003: Add --with-native-debug-symbols=[none|internal|external|zipped]
> The use of these two conditionals seems odd to me. What we want to know
> is whether zipped debuginfo is in use.
No. We want to know whether or not external debug symbols (zipped or
otherwise) are in use.
> This is not the same as debug symbols
> in general being enabled.
Yes. Correctly so.
> Also, DEBUGINFO_EXT is only being set when
> ZIP_DEBUGINFO_FILES is true!
How so?
ifeq ($(ZIP_DEBUGINFO_FILES), true)
DEBUGINFO_EXT := .diz
else ifeq ($(OPENJDK_TARGET_OS), macosx)
DEBUGINFO_EXT := .dSYM
else ifeq ($(OPENJDK_TARGET_OS), windows)
DEBUGINFO_EXT := .pdb
else
DEBUGINFO_EXT := .debuginfo
endif
On Linux with --with-native-debug-symbols=external this ends up with:
DEBUGINFO_EXT := .debuginfo
> POST_STRIP_CMD seems to only be used in image creation, so I don't
> think checking this is appropriate either.
Seems debatable.
> Instead, we should mirror what is done in make/common/NativeCompilation.gmk
> when
> creating the .diz files:
>
> -ifeq ($(ENABLE_DEBUG_SYMBOLS), true)
> - ifneq ($(POST_STRIP_CMD), )
> +ifeq ($(ZIP_DEBUGINFO_FILES), true)
> + ifneq ($(STRIP_POLICY), no_strip)
>
> The second check is necessary because ZIP_DEBUGINFO_FILES is set by default,
> and so may be true even if STRIP_POLICY has been set to no_strip.
Your patch won't work for --with-native-debug-symbols=external. In that
case no *.debuginfo files for the java and unpack200 launchers would be
created.
With your patch:
$ find build/linux-x86_64-normal-server-release/images/j2sdk-image/bin/ | grep
-E 'java\.debuginfo|unpack200\.debuginfo'
Expected:
$ find build/linux-x86_64-normal-server-release/images/j2sdk-image/bin/ | grep
-E 'java\.debuginfo|unpack200\.debuginfo'
build/linux-x86_64-normal-server-release/images/j2sdk-image/bin/java.debuginfo
build/linux-x86_64-normal-server-release/images/j2sdk-image/bin/unpack200.debuginfo
> The attached patch fixed the build for me.
Sorry, I don't seem to be able to reproduce your build failure.
Thanks,
Severin
Re: RFR: JDK-8253239: Disable VS warning C4307
On Fri, 18 Sep 2020 08:22:56 GMT, Matthias Baesken wrote: > hello, this fixes the build when using VS2017. VS2019 does not have the issue > as far as I know. > failure was > >> ./test/hotspot/gtest/utilities/test_align.cpp(96): error C2220: warning >> treated as error - no 'object' file generated >> ./test/hotspot/gtest/utilities/test_align.cpp(156): note: see reference to >> function template instantiation 'void >> static_test_alignments(void)' being compiled with >> [ >> T=int64_t, >> A=uint8_t >> ] >> ./test/hotspot/gtest/utilities/test_align.cpp(162): note: see reference to >> function template instantiation 'void >> test_alignments(void)' being compiled >> ./test/hotspot/gtest/utilities/test_align.cpp(96): warning >> C4307: '+': integral constant overflow >> >> > > This might be related to an issue fixed at least in some versions of VS2019 , > that is discussed here : > > https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html > > https://bugs.openjdk.java.net/browse/JDK-8253239 make/autoconf/flags-cflags.m4 line 138: > 136: DISABLED_WARNINGS="4800" > 137: if test "x$TOOLCHAIN_VERSION" = x2017; then > 138: DISABLED_WARNINGS+=" 4307" Seems like this file uses 2 spaces as indenting, 4 spaces are used here. - PR: https://git.openjdk.java.net/jdk/pull/237
RFR: JDK-8253239: Disable VS warning C4307
hello, this fixes the build when using VS2017. VS2019 does not have the issue as far as I know. failure was > ./test/hotspot/gtest/utilities/test_align.cpp(96): error C2220: warning > treated as error - no 'object' file generated > ./test/hotspot/gtest/utilities/test_align.cpp(156): note: see reference to > function template instantiation 'void > static_test_alignments(void)' being compiled with > [ > T=int64_t, > A=uint8_t > ] > ./test/hotspot/gtest/utilities/test_align.cpp(162): note: see reference to > function template instantiation 'void > test_alignments(void)' being compiled > ./test/hotspot/gtest/utilities/test_align.cpp(96): warning > C4307: '+': integral constant overflow > > This might be related to an issue fixed at least in some versions of VS2019 , that is discussed here : https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html https://bugs.openjdk.java.net/browse/JDK-8253239 - Commit messages: - JDK-8253239 Changes: https://git.openjdk.java.net/jdk/pull/237/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=237=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253239 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/237.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/237/head:pull/237 PR: https://git.openjdk.java.net/jdk/pull/237
