Re: [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246]

[Iain Sandoe <iain@sandoe.co.uk>]

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
>