Message ID | 20211104100528.GS304296@tucnak |
---|---|
State | Committed |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7FB9A3857C60 for <patchwork@sourceware.org>; Thu, 4 Nov 2021 10:06:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7FB9A3857C60 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636020365; bh=fkKenmIAQY996nI3F7rXCxzv85/X+h85MbAGxEW6uoo=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=H2/rrMMdU7UIxj2g+UM42rDZM8VYQxA6GbcnkWcLu/kAualz05jj8DkA8tTtni34V tCc99lZepziuIAggVPCzZCbbgvM5ymX9TK6ZtJv4x9S4xS0+q7udBtJefU1SgK+3ob Fv/1HaBj5ARr8xl152kRFHqLLZOs+oimZzhT6DVE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTPS id 4E4CA3858409 for <gcc-patches@gcc.gnu.org>; Thu, 4 Nov 2021 10:05:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4E4CA3858409 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-593-F8Id7q56PSqjcShK7Piyxw-1; Thu, 04 Nov 2021 06:05:35 -0400 X-MC-Unique: F8Id7q56PSqjcShK7Piyxw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E5D0B19057A1; Thu, 4 Nov 2021 10:05:33 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.193.172]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7F3D170871; Thu, 4 Nov 2021 10:05:33 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 1A4A5T753342796 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 4 Nov 2021 11:05:30 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 1A4A5SIR3342793; Thu, 4 Nov 2021 11:05:28 +0100 Date: Thu, 4 Nov 2021 11:05:28 +0100 To: Iain Sandoe <iain@sandoe.co.uk>, Uros Bizjak <ubizjak@gmail.com>, Jan Hubicka <jh@suse.cz> Subject: [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246] Message-ID: <20211104100528.GS304296@tucnak> References: <B933DF7C-5188-473C-AC9A-1DD9EBF0E573@sandoe.co.uk> <CAFiYyc2DL6SKO8rwXJs12pFiH=cwnfUJ+6ya=voZ_DYTe9Lrbw@mail.gmail.com> <C3B91E3D-7D47-470D-9378-8B4B8173AE79@sandoe.co.uk> <CAFiYyc0YZ-CWo_z7X-UeV04_-pBBkSvJJZYPoGoN3mvcHK89=g@mail.gmail.com> <2AF05BB0-7510-4B7B-8702-8AE76EC622E6@sandoe.co.uk> MIME-Version: 1.0 In-Reply-To: <2AF05BB0-7510-4B7B-8702-8AE76EC622E6@sandoe.co.uk> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Jakub Jelinek <jakub@redhat.com> Cc: GCC Patches <gcc-patches@gcc.gnu.org> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246]
|
|
Commit Message
Jakub Jelinek
Nov. 4, 2021, 10:05 a.m. UTC
On Tue, Jul 06, 2021 at 11:17:55AM +0100, Iain Sandoe wrote: > >> The addition of the CTOR is the fix for the C++ compile fail in the PR, the conditional is > >> only there because the same header is compiled by C and C++. > > > > Whoops sorry - I was confused. The patch looks OK to me if you add a comment > > before the CTOR why it was added (maybe quoting the error that happens) > > Thanks, pushed as below. > Iain > > ------- > > X86: Provide a CTOR for stringop_algs [PR100246]. > > Several older compilers fail to build modern GCC because of missing > or incomplete C++11 support. > > Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> > > PR bootstrap/100246 - [11/12 Regression] GCC will not bootstrap with clang 3.4/3.5 [xcode 5/6, Darwin 12/13] > > PR bootstrap/100246 > > gcc/ChangeLog: > > * config/i386/i386.h (struct stringop_algs): Define a CTOR for > this type. Unfortunately, as mentioned in my https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583289.html mail, without the new dyninit pass this causes dynamic initialization of many variables, 6.5KB _GLOBAL__sub_I_* on x86_64 and 12.5KB on i686. The following so far only lightly tested patch makes the ctor constexpr so that already the FE is able to statically initialize all those. I don't have access to Darwin nor to the broken versions of clang, do you think you could test bootstrap there with this too? Especially because 11.x is not going to have the dyninit optimization for sure, it would be nice to do this on the 11 branch too. 2021-11-04 Jakub Jelinek <jakub@redhat.com> PR bootstrap/100246 * config/i386/i386.h (stringop_algs::stringop_strategy::stringop_strategy): Make the ctor constexpr. Jakub
Comments
Hi Jakub, > On 4 Nov 2021, at 10:05, Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Jul 06, 2021 at 11:17:55AM +0100, Iain Sandoe wrote: >>>> The addition of the CTOR is the fix for the C++ compile fail in the PR, the conditional is >>>> only there because the same header is compiled by C and C++. >>> >>> Whoops sorry - I was confused. The patch looks OK to me if you add a comment >>> before the CTOR why it was added (maybe quoting the error that happens) >> >> Thanks, pushed as below. >> Iain >> >> ------- >> >> X86: Provide a CTOR for stringop_algs [PR100246]. >> >> Several older compilers fail to build modern GCC because of missing >> or incomplete C++11 support. >> >> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> >> >> PR bootstrap/100246 - [11/12 Regression] GCC will not bootstrap with clang 3.4/3.5 [xcode 5/6, Darwin 12/13] >> >> PR bootstrap/100246 >> >> gcc/ChangeLog: >> >> * config/i386/i386.h (struct stringop_algs): Define a CTOR for >> this type. > > Unfortunately, as mentioned in my > https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583289.html > mail, without the new dyninit pass this causes dynamic initialization of > many variables, 6.5KB _GLOBAL__sub_I_* on x86_64 and 12.5KB on i686. > > The following so far only lightly tested patch makes the ctor constexpr > so that already the FE is able to statically initialize all those. > > I don't have access to Darwin nor to the broken versions of clang, do you > think you could test bootstrap there with this too? Bootstrap succeeded with Apple clang-503.0.40 (Xcode 5.1.1) on macOS 10.8 which is the earliest version I expect to work (previous xcode impl. have more C++11 incompatibilities). So OK from a Darwin PoV. The other reported toolchain with the issue was GCC-4.9.2 as discussed on IRC - this also seems OK. Iain > Especially because 11.x is not going to have the dyninit optimization for > sure, it would be nice to do this on the 11 branch too. > > 2021-11-04 Jakub Jelinek <jakub@redhat.com> > > PR bootstrap/100246 > * config/i386/i386.h > (stringop_algs::stringop_strategy::stringop_strategy): Make the ctor > constexpr. > > --- gcc/config/i386/i386.h.jj 2021-09-28 23:18:35.282563395 +0200 > +++ gcc/config/i386/i386.h 2021-11-04 10:48:47.165086806 +0100 > @@ -78,8 +78,9 @@ struct stringop_algs > this issue. Since this header is used by code compiled with the C > compiler we must guard the addition. */ > #ifdef __cplusplus > - stringop_strategy(int _max = -1, enum stringop_alg _alg = libcall, > - int _noalign = false) > + constexpr stringop_strategy(int _max = -1, > + enum stringop_alg _alg = libcall, > + int _noalign = false) > : max (_max), alg (_alg), noalign (_noalign) {} > #endif > const int max; > > Jakub >
On Thu, Nov 04, 2021 at 12:39:34PM +0000, Iain Sandoe wrote: > Bootstrap succeeded with Apple clang-503.0.40 (Xcode 5.1.1) on macOS 10.8 > which is the earliest version I expect to work (previous xcode impl. have more > C++11 incompatibilities). So OK from a Darwin PoV. > > The other reported toolchain with the issue was GCC-4.9.2 as discussed on > IRC - this also seems OK. Yeah, I've been testing it on a short testcase with just enum stringop_alg, struct stringop_algs and ix86_size_memcpy on godbolt too: https://godbolt.org/z/vfcz8xen6 enum stringop_alg { no_stringop, libcall, rep_prefix_1_byte, rep_prefix_4_byte, rep_prefix_8_byte, loop_1_byte, loop, unrolled_loop, vector_loop, last_alg }; struct stringop_algs { const enum stringop_alg unknown_size; const struct stringop_strategy { #ifndef NO_CTOR #ifdef CONSTEXPR constexpr #endif stringop_strategy(int _max = -1, enum stringop_alg _alg = libcall, int _noalign = false) : max (_max), alg (_alg), noalign (_noalign) {} #endif const int max; const enum stringop_alg alg; int noalign; } size [4]; }; stringop_algs ix86_size_memcpy[2] = { {rep_prefix_1_byte, {{-1, rep_prefix_1_byte, false}}}, {rep_prefix_1_byte, {{-1, rep_prefix_1_byte, false}}}}; and tested the various cases, no stringop_strategy ctor at all, the ctor and ctor with constexpr. clang before 3.3 is unhappy about all the 3 cases, clang 3.3 and 3.4 is ok with ctor and ctor with constexpr and optimizes it into static initialization, clang 3.5+ is ok with all 3 versions and optimizes, gcc 4.8 and 5+ is ok with all 3 versions and no ctor and ctor with constexpr is optimized, gcc 4.9 is unhappy about the no ctor case and happy with the other two. > > Especially because 11.x is not going to have the dyninit optimization for > > sure, it would be nice to do this on the 11 branch too. > > > > 2021-11-04 Jakub Jelinek <jakub@redhat.com> > > > > PR bootstrap/100246 > > * config/i386/i386.h > > (stringop_algs::stringop_strategy::stringop_strategy): Make the ctor > > constexpr. > > > > --- gcc/config/i386/i386.h.jj 2021-09-28 23:18:35.282563395 +0200 > > +++ gcc/config/i386/i386.h 2021-11-04 10:48:47.165086806 +0100 > > @@ -78,8 +78,9 @@ struct stringop_algs > > this issue. Since this header is used by code compiled with the C > > compiler we must guard the addition. */ > > #ifdef __cplusplus > > - stringop_strategy(int _max = -1, enum stringop_alg _alg = libcall, > > - int _noalign = false) > > + constexpr stringop_strategy(int _max = -1, > > + enum stringop_alg _alg = libcall, > > + int _noalign = false) > > : max (_max), alg (_alg), noalign (_noalign) {} > > #endif > > const int max; Jakub
On Thu, Nov 04, 2021 at 01:45:38PM +0100, Jakub Jelinek via Gcc-patches wrote: > On Thu, Nov 04, 2021 at 12:39:34PM +0000, Iain Sandoe wrote: > > Bootstrap succeeded with Apple clang-503.0.40 (Xcode 5.1.1) on macOS 10.8 > > which is the earliest version I expect to work (previous xcode impl. have more > > C++11 incompatibilities). So OK from a Darwin PoV. > > > > The other reported toolchain with the issue was GCC-4.9.2 as discussed on > > IRC - this also seems OK. > > > > Especially because 11.x is not going to have the dyninit optimization for > > > sure, it would be nice to do this on the 11 branch too. Bootstrapped/regtested on x86_64-linux and i686-linux successfully too, with sligtly different formatting, as I think in our coding style constexpr should go on the previous line and the ctor didn't have space before (. Ok for trunk and 11.3? 2021-11-05 Jakub Jelinek <jakub@redhat.com> PR bootstrap/100246 * config/i386/i386.h (stringop_algs::stringop_strategy::stringop_strategy): Make the ctor constexpr. --- gcc/config/i386/i386.h.jj 2021-09-28 23:18:35.282563395 +0200 +++ gcc/config/i386/i386.h 2021-11-04 10:48:47.165086806 +0100 @@ -78,8 +78,9 @@ struct stringop_algs this issue. Since this header is used by code compiled with the C compiler we must guard the addition. */ #ifdef __cplusplus - stringop_strategy(int _max = -1, enum stringop_alg _alg = libcall, - int _noalign = false) + constexpr + stringop_strategy (int _max = -1, enum stringop_alg _alg = libcall, + int _noalign = false) : max (_max), alg (_alg), noalign (_noalign) {} #endif const int max; Jakub
On Fri, Nov 5, 2021 at 10:59 AM Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On Thu, Nov 04, 2021 at 01:45:38PM +0100, Jakub Jelinek via Gcc-patches wrote: > > On Thu, Nov 04, 2021 at 12:39:34PM +0000, Iain Sandoe wrote: > > > Bootstrap succeeded with Apple clang-503.0.40 (Xcode 5.1.1) on macOS 10.8 > > > which is the earliest version I expect to work (previous xcode impl. have more > > > C++11 incompatibilities). So OK from a Darwin PoV. > > > > > > The other reported toolchain with the issue was GCC-4.9.2 as discussed on > > > IRC - this also seems OK. > > > > > > Especially because 11.x is not going to have the dyninit optimization for > > > > sure, it would be nice to do this on the 11 branch too. > > Bootstrapped/regtested on x86_64-linux and i686-linux successfully too, with > sligtly different formatting, as I think in our coding style constexpr > should go on the previous line and the ctor didn't have space before (. > > Ok for trunk and 11.3? OK. > 2021-11-05 Jakub Jelinek <jakub@redhat.com> > > PR bootstrap/100246 > * config/i386/i386.h > (stringop_algs::stringop_strategy::stringop_strategy): Make the ctor > constexpr. > > --- gcc/config/i386/i386.h.jj 2021-09-28 23:18:35.282563395 +0200 > +++ gcc/config/i386/i386.h 2021-11-04 10:48:47.165086806 +0100 > @@ -78,8 +78,9 @@ struct stringop_algs > this issue. Since this header is used by code compiled with the C > compiler we must guard the addition. */ > #ifdef __cplusplus > - stringop_strategy(int _max = -1, enum stringop_alg _alg = libcall, > - int _noalign = false) > + constexpr > + stringop_strategy (int _max = -1, enum stringop_alg _alg = libcall, > + int _noalign = false) > : max (_max), alg (_alg), noalign (_noalign) {} > #endif > const int max; > > > Jakub >
--- gcc/config/i386/i386.h.jj 2021-09-28 23:18:35.282563395 +0200 +++ gcc/config/i386/i386.h 2021-11-04 10:48:47.165086806 +0100 @@ -78,8 +78,9 @@ struct stringop_algs this issue. Since this header is used by code compiled with the C compiler we must guard the addition. */ #ifdef __cplusplus - stringop_strategy(int _max = -1, enum stringop_alg _alg = libcall, - int _noalign = false) + constexpr stringop_strategy(int _max = -1, + enum stringop_alg _alg = libcall, + int _noalign = false) : max (_max), alg (_alg), noalign (_noalign) {} #endif const int max;