Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Tue, 19 Dec 2023 23:23:28 GMT, Kim Barrett wrote: >>> @lgxbslgx We would like to keep GCC 8.4.0 as the minimum. >> >> Why? That's likely going to be in conflict with >> https://github.com/openjdk/jdk/pull/14988. > >> @kimbarrett I meant to say that since the libsimdsort works with GCC 8.4.0, >> the #define guard in libsimdsort sources could be restricted to just that >> and we don't have to artificially raise it to GCC 9. Do you think that is an >> issue? > > I don't think we should be depending on an experimental compiler feature when > there are alternatives. @kimbarrett @sviswa7 Thanks for your reviews. Integrating. - PR Comment: https://git.openjdk.org/jdk/pull/17047#issuecomment-1863808489
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Sun, 17 Dec 2023 13:25:00 GMT, Guoxiong Li wrote: >> Hi all, >> >> This patch fixes the building failure introduced by >> [JDK-8319577](https://bugs.openjdk.org/browse/JDK-8319577) in old GCC >> version (linux & GCC 7.5.0 locally). >> >> Thanks for the review. >> >> Best Regards, >> -- Guoxiong > > Guoxiong Li has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains four additional > commits since the last revision: > > - Bump the needed version of GCC. > - Revert previous change. > - Merge branch 'master' into JDK-8321688 > - JDK-8321688 Marked as reviewed by sviswanathan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17047#pullrequestreview-1789880110
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Sun, 17 Dec 2023 13:25:00 GMT, Guoxiong Li wrote: >> Hi all, >> >> This patch fixes the building failure introduced by >> [JDK-8319577](https://bugs.openjdk.org/browse/JDK-8319577) in old GCC >> version (linux & GCC 7.5.0 locally). >> >> Thanks for the review. >> >> Best Regards, >> -- Guoxiong > > Guoxiong Li has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains four additional > commits since the last revision: > > - Bump the needed version of GCC. > - Revert previous change. > - Merge branch 'master' into JDK-8321688 > - JDK-8321688 Looks good. - Marked as reviewed by kbarrett (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17047#pullrequestreview-1789856820
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Tue, 19 Dec 2023 19:08:08 GMT, Kim Barrett wrote: >>> Have you tested with gcc 9? Or is this just supposition based on gcc9 >>> having removed the experimental >> status for C++17? >> >> I have not tested GCC 8 and 9. @sviswa7 seems to test them. >> >>> I have verified that with the above change the builds (release, fastdebug, >>> slowdebug) all succeed with GCC 8.4.0 as well as prior GCC like GCC7.5.0 >>> and the test/jdk/java/util/Arrays/Sorting.java passes successfully with >>> these builds. >> >> Thanks for your tests. But from the description of the [GCC >> document](https://gcc.gnu.org/projects/cxx-status.html), shown below, it may >> be good to skip GCC 8 and use GCC 9 directly if we want to switch to C++17. >> >>> Some C++17 features are available since GCC 5, but support was >>> experimental and the ABI of C++17 features was not stable until GCC 9. >> >> What do you think about it? > >> @lgxbslgx We would like to keep GCC 8.4.0 as the minimum. > > Why? That's likely going to be in conflict with > https://github.com/openjdk/jdk/pull/14988. > @kimbarrett I meant to say that since the libsimdsort works with GCC 8.4.0, > the #define guard in libsimdsort sources could be restricted to just that and > we don't have to artificially raise it to GCC 9. Do you think that is an > issue? I don't think we should be depending on an experimental compiler feature when there are alternatives. - PR Comment: https://git.openjdk.org/jdk/pull/17047#issuecomment-1863608489
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Tue, 19 Dec 2023 19:08:08 GMT, Kim Barrett wrote: >>> Have you tested with gcc 9? Or is this just supposition based on gcc9 >>> having removed the experimental >> status for C++17? >> >> I have not tested GCC 8 and 9. @sviswa7 seems to test them. >> >>> I have verified that with the above change the builds (release, fastdebug, >>> slowdebug) all succeed with GCC 8.4.0 as well as prior GCC like GCC7.5.0 >>> and the test/jdk/java/util/Arrays/Sorting.java passes successfully with >>> these builds. >> >> Thanks for your tests. But from the description of the [GCC >> document](https://gcc.gnu.org/projects/cxx-status.html), shown below, it may >> be good to skip GCC 8 and use GCC 9 directly if we want to switch to C++17. >> >>> Some C++17 features are available since GCC 5, but support was >>> experimental and the ABI of C++17 features was not stable until GCC 9. >> >> What do you think about it? > >> @lgxbslgx We would like to keep GCC 8.4.0 as the minimum. > > Why? That's likely going to be in conflict with > https://github.com/openjdk/jdk/pull/14988. @kimbarrett I meant to say that since the libsimdsort works with GCC 8.4.0, the #define guard in libsimdsort sources could be restricted to just that and we don't have to artificially raise it to GCC 9. Do you think that is an issue? - PR Comment: https://git.openjdk.org/jdk/pull/17047#issuecomment-1863367786
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Tue, 19 Dec 2023 02:22:05 GMT, Guoxiong Li wrote: >> Guoxiong Li has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains four additional >> commits since the last revision: >> >> - Bump the needed version of GCC. >> - Revert previous change. >> - Merge branch 'master' into JDK-8321688 >> - JDK-8321688 > >> Have you tested with gcc 9? Or is this just supposition based on gcc9 having >> removed the experimental > status for C++17? > > I have not tested GCC 8 and 9. @sviswa7 seems to test them. > >> I have verified that with the above change the builds (release, fastdebug, >> slowdebug) all succeed with GCC 8.4.0 as well as prior GCC like GCC7.5.0 and >> the test/jdk/java/util/Arrays/Sorting.java passes successfully with these >> builds. > > Thanks for your tests. But from the description of the [GCC > document](https://gcc.gnu.org/projects/cxx-status.html), shown below, it may > be good to skip GCC 8 and use GCC 9 directly if we want to switch to C++17. > >> Some C++17 features are available since GCC 5, but support was experimental >> and the ABI of C++17 features was not stable until GCC 9. > > What do you think about it? > @lgxbslgx We would like to keep GCC 8.4.0 as the minimum. Why? That's likely going to be in conflict with https://github.com/openjdk/jdk/pull/14988. - PR Comment: https://git.openjdk.org/jdk/pull/17047#issuecomment-1863330523
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Tue, 19 Dec 2023 02:22:05 GMT, Guoxiong Li wrote: >> Guoxiong Li has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains four additional >> commits since the last revision: >> >> - Bump the needed version of GCC. >> - Revert previous change. >> - Merge branch 'master' into JDK-8321688 >> - JDK-8321688 > >> Have you tested with gcc 9? Or is this just supposition based on gcc9 having >> removed the experimental > status for C++17? > > I have not tested GCC 8 and 9. @sviswa7 seems to test them. > >> I have verified that with the above change the builds (release, fastdebug, >> slowdebug) all succeed with GCC 8.4.0 as well as prior GCC like GCC7.5.0 and >> the test/jdk/java/util/Arrays/Sorting.java passes successfully with these >> builds. > > Thanks for your tests. But from the description of the [GCC > document](https://gcc.gnu.org/projects/cxx-status.html), shown below, it may > be good to skip GCC 8 and use GCC 9 directly if we want to switch to C++17. > >> Some C++17 features are available since GCC 5, but support was experimental >> and the ABI of C++17 features was not stable until GCC 9. > > What do you think about it? @lgxbslgx We would like to keep GCC 8.4.0 as the minimum. - PR Comment: https://git.openjdk.org/jdk/pull/17047#issuecomment-1863127078
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Sun, 17 Dec 2023 13:25:00 GMT, Guoxiong Li wrote: >> Hi all, >> >> This patch fixes the building failure introduced by >> [JDK-8319577](https://bugs.openjdk.org/browse/JDK-8319577) in old GCC >> version (linux & GCC 7.5.0 locally). >> >> Thanks for the review. >> >> Best Regards, >> -- Guoxiong > > Guoxiong Li has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains four additional > commits since the last revision: > > - Bump the needed version of GCC. > - Revert previous change. > - Merge branch 'master' into JDK-8321688 > - JDK-8321688 > Have you tested with gcc 9? Or is this just supposition based on gcc9 having > removed the experimental status for C++17? I have not tested GCC 8 and 9. @sviswa7 seems to test them. > I have verified that with the above change the builds (release, fastdebug, > slowdebug) all succeed with GCC 8.4.0 as well as prior GCC like GCC7.5.0 and > the test/jdk/java/util/Arrays/Sorting.java passes successfully with these > builds. Thanks for your tests. But from the description of the [GCC document](https://gcc.gnu.org/projects/cxx-status.html), shown below, it may be good to skip GCC 8 and use GCC 9 directly if we want to switch to C++17. > Some C++17 features are available since GCC 5, but support was experimental > and the ABI of C++17 features was not stable until GCC 9. What do you think about it? - PR Comment: https://git.openjdk.org/jdk/pull/17047#issuecomment-1861998852
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Sun, 17 Dec 2023 13:25:00 GMT, Guoxiong Li wrote:
>> Hi all,
>>
>> This patch fixes the building failure introduced by
>> [JDK-8319577](https://bugs.openjdk.org/browse/JDK-8319577) in old GCC
>> version (linux & GCC 7.5.0 locally).
>>
>> Thanks for the review.
>>
>> Best Regards,
>> -- Guoxiong
>
> Guoxiong Li has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains four additional
> commits since the last revision:
>
> - Bump the needed version of GCC.
> - Revert previous change.
> - Merge branch 'master' into JDK-8321688
> - JDK-8321688
Let us restrict the libsimdsort.so build to GCC 8.4.0 with the following change:
`diff --git a/src/java.base/linux/native/libsimdsort/simdsort-support.hpp
b/src/java.base/linux/native/libsimdsort/simdsort-support.hpp
index c73fd7920d8..2a2946cff82 100644
--- a/src/java.base/linux/native/libsimdsort/simdsort-support.hpp
+++ b/src/java.base/linux/native/libsimdsort/simdsort-support.hpp
@@ -31,9 +31,9 @@
#define assert(cond, msg) { if (!(cond)) { fprintf(stderr, "assert fails %s
%d: %s\n", __FILE__, __LINE__, msg); abort(); }}
-// GCC >= 7.5 is needed to build AVX2 portions of libsimdsort using C++17
features
-#if defined(_LP64) && (defined(__GNUC__) && ((__GNUC__ > 7) || ((__GNUC__ ==
7) && (__GNUC_MINOR__ >= 5
+// GCC >= 8.4 is needed to build AVX2 portions of libsimdsort using C++17
features
+#if defined(_LP64) && (defined(__GNUC__) && ((__GNUC__ > 8) || ((__GNUC__ ==
8) && (__GNUC_MINOR__ >= 4
#define __SIMDSORT_SUPPORTED_LINUX
#endif
-#endif //SIMDSORT_SUPPORT_HPP
\ No newline at end of file
+#endif //SIMDSORT_SUPPORT_HPP`
I have verified that with the above change the builds (release, fastdebug,
slowdebug) all succeed with GCC 8.4.0 as well as prior GCC like GCC7.5.0 and
the test/jdk/java/util/Arrays/Sorting.java passes successfully with these
builds.
-
PR Comment: https://git.openjdk.org/jdk/pull/17047#issuecomment-1861889879
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
On Sun, 17 Dec 2023 13:25:00 GMT, Guoxiong Li wrote: >> Hi all, >> >> This patch fixes the building failure introduced by >> [JDK-8319577](https://bugs.openjdk.org/browse/JDK-8319577) in old GCC >> version (linux & GCC 7.5.0 locally). >> >> Thanks for the review. >> >> Best Regards, >> -- Guoxiong > > Guoxiong Li has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains four additional > commits since the last revision: > > - Bump the needed version of GCC. > - Revert previous change. > - Merge branch 'master' into JDK-8321688 > - JDK-8321688 src/java.base/linux/native/libsimdsort/simdsort-support.hpp line 35: > 33: > 34: // GCC >= 9.1 is needed to build AVX2 portions of libsimdsort using C++17 > features > 35: #if defined(_LP64) && (defined(__GNUC__) && ((__GNUC__ > 9) || ((__GNUC__ > == 9) && (__GNUC_MINOR__ >= 1 Have you tested with gcc 9? Or is this just supposition based on gcc9 having removed the experimental status for C++17? - PR Review Comment: https://git.openjdk.org/jdk/pull/17047#discussion_r1430308457
Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577 [v2]
> Hi all, > > This patch fixes the building failure introduced by > [JDK-8319577](https://bugs.openjdk.org/browse/JDK-8319577) in old GCC version > (linux & GCC 7.5.0 locally). > > Thanks for the review. > > Best Regards, > -- Guoxiong Guoxiong Li has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - Bump the needed version of GCC. - Revert previous change. - Merge branch 'master' into JDK-8321688 - JDK-8321688 - Changes: - all: https://git.openjdk.org/jdk/pull/17047/files - new: https://git.openjdk.org/jdk/pull/17047/files/46858210..f04bcd55 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17047=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17047=00-01 Stats: 876 lines in 56 files changed: 520 ins; 166 del; 190 mod Patch: https://git.openjdk.org/jdk/pull/17047.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17047/head:pull/17047 PR: https://git.openjdk.org/jdk/pull/17047
