public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] X86: Provide a CTOR for stringop_algs [PR100246].
@ 2021-07-04 20:03 Iain Sandoe
  2021-07-05 10:50 ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Iain Sandoe @ 2021-07-04 20:03 UTC (permalink / raw)
  To: GCC Patches

Hi,

Several older compilers fail to build modern GCC because of missing
or incomplete C++11 support.

(although the PR mentions clang, specifically, this has also been reported
 for some GCC versions within the range that should be able to bootstrap
 GCC)

There are several possible solutions proposed in the PR, this one seems
 the least invasive.

The header is pulled into the gcov code that builds with C, so we have to
make the CTOR conditional on C++.

tested on Darwin12 with xcode-6, bootstrapped on x86_64-darwin and linux.
OK for master / GCC-11?
thanks
Iain

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.
---
 gcc/config/i386/i386.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 6e0340a4b60..84151156999 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -73,6 +73,11 @@ struct stringop_algs
 {
   const enum stringop_alg unknown_size;
   const struct stringop_strategy {
+#ifdef __cplusplus
+    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;
-- 
2.24.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] X86: Provide a CTOR for stringop_algs [PR100246].
  2021-07-04 20:03 [PATCH] X86: Provide a CTOR for stringop_algs [PR100246] Iain Sandoe
@ 2021-07-05 10:50 ` Richard Biener
  2021-07-05 13:04   ` Iain Sandoe
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-07-05 10:50 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches

On Sun, Jul 4, 2021 at 10:04 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Hi,
>
> Several older compilers fail to build modern GCC because of missing
> or incomplete C++11 support.
>
> (although the PR mentions clang, specifically, this has also been reported
>  for some GCC versions within the range that should be able to bootstrap
>  GCC)
>
> There are several possible solutions proposed in the PR, this one seems
>  the least invasive.
>
> The header is pulled into the gcov code that builds with C, so we have to
> make the CTOR conditional on C++.
>
> tested on Darwin12 with xcode-6, bootstrapped on x86_64-darwin and linux.
> OK for master / GCC-11?

Hmm, what is specifically built with a C compiler?  gcov.c not, I think.

Instead of commenting the CTOR, does it work to comment the whole stringop_algs
type?  Also it seems on trunk this CTOR is no more?

> thanks
> Iain
>
> 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.
> ---
>  gcc/config/i386/i386.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> index 6e0340a4b60..84151156999 100644
> --- a/gcc/config/i386/i386.h
> +++ b/gcc/config/i386/i386.h
> @@ -73,6 +73,11 @@ struct stringop_algs
>  {
>    const enum stringop_alg unknown_size;
>    const struct stringop_strategy {
> +#ifdef __cplusplus
> +    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;
> --
> 2.24.1
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] X86: Provide a CTOR for stringop_algs [PR100246].
  2021-07-05 10:50 ` Richard Biener
@ 2021-07-05 13:04   ` Iain Sandoe
  2021-07-05 13:23     ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Iain Sandoe @ 2021-07-05 13:04 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

Hi Richard,

> On 5 Jul 2021, at 11:50, Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> On Sun, Jul 4, 2021 at 10:04 PM Iain Sandoe <iain@sandoe.co.uk> wrote:

>> Several older compilers fail to build modern GCC because of missing
>> or incomplete C++11 support.
>> 
>> (although the PR mentions clang, specifically, this has also been reported
>> for some GCC versions within the range that should be able to bootstrap
>> GCC)
>> 
>> There are several possible solutions proposed in the PR, this one seems
>> the least invasive.
>> 
>> The header is pulled into the gcov code that builds with C, so we have to
>> make the CTOR conditional on C++.
>> 
>> tested on Darwin12 with xcode-6, bootstrapped on x86_64-darwin and linux.
>> OK for master / GCC-11?
> 
> Hmm, what is specifically built with a C compiler?  gcov.c not, I think.

any C compilation that includes tm.h

well, libgcc2 fails too on a quick check here -  but ISTR there was something in
libgcov and I checked with Martin that it was intentionally compiled with C compiler.

> Instead of commenting the CTOR, does it work to comment the whole stringop_algs
> type?

I don’t think that will work because it’s in a header that’s transitively included by tm.h
which is then included loads of places.

>  Also it seems on trunk this CTOR is no more?

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++.

thanks
Iain
> 
>> thanks
>> Iain
>> 
>> 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.
>> ---
>> gcc/config/i386/i386.h | 5 +++++
>> 1 file changed, 5 insertions(+)
>> 
>> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
>> index 6e0340a4b60..84151156999 100644
>> --- a/gcc/config/i386/i386.h
>> +++ b/gcc/config/i386/i386.h
>> @@ -73,6 +73,11 @@ struct stringop_algs
>> {
>>   const enum stringop_alg unknown_size;
>>   const struct stringop_strategy {
>> +#ifdef __cplusplus
>> +    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;
>> --
>> 2.24.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] X86: Provide a CTOR for stringop_algs [PR100246].
  2021-07-05 13:04   ` Iain Sandoe
@ 2021-07-05 13:23     ` Richard Biener
  2021-07-06 10:17       ` Iain Sandoe
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-07-05 13:23 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches

On Mon, Jul 5, 2021 at 3:04 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Hi Richard,
>
> > On 5 Jul 2021, at 11:50, Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >
> > On Sun, Jul 4, 2021 at 10:04 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> >> Several older compilers fail to build modern GCC because of missing
> >> or incomplete C++11 support.
> >>
> >> (although the PR mentions clang, specifically, this has also been reported
> >> for some GCC versions within the range that should be able to bootstrap
> >> GCC)
> >>
> >> There are several possible solutions proposed in the PR, this one seems
> >> the least invasive.
> >>
> >> The header is pulled into the gcov code that builds with C, so we have to
> >> make the CTOR conditional on C++.
> >>
> >> tested on Darwin12 with xcode-6, bootstrapped on x86_64-darwin and linux.
> >> OK for master / GCC-11?
> >
> > Hmm, what is specifically built with a C compiler?  gcov.c not, I think.
>
> any C compilation that includes tm.h
>
> well, libgcc2 fails too on a quick check here -  but ISTR there was something in
> libgcov and I checked with Martin that it was intentionally compiled with C compiler.
>
> > Instead of commenting the CTOR, does it work to comment the whole stringop_algs
> > type?
>
> I don’t think that will work because it’s in a header that’s transitively included by tm.h
> which is then included loads of places.
>
> >  Also it seems on trunk this CTOR is no more?
>
> 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)

Richard.

> thanks
> Iain
> >
> >> thanks
> >> Iain
> >>
> >> 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.
> >> ---
> >> gcc/config/i386/i386.h | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> >> index 6e0340a4b60..84151156999 100644
> >> --- a/gcc/config/i386/i386.h
> >> +++ b/gcc/config/i386/i386.h
> >> @@ -73,6 +73,11 @@ struct stringop_algs
> >> {
> >>   const enum stringop_alg unknown_size;
> >>   const struct stringop_strategy {
> >> +#ifdef __cplusplus
> >> +    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;
> >> --
> >> 2.24.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] X86: Provide a CTOR for stringop_algs [PR100246].
  2021-07-05 13:23     ` Richard Biener
@ 2021-07-06 10:17       ` Iain Sandoe
  2021-11-04 10:05         ` [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246] Jakub Jelinek
  0 siblings, 1 reply; 10+ messages in thread
From: Iain Sandoe @ 2021-07-06 10:17 UTC (permalink / raw)
  To: GCC Patches


> On 5 Jul 2021, at 14:23, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Mon, Jul 5, 2021 at 3:04 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>> 
>> Hi Richard,
>> 
>>> On 5 Jul 2021, at 11:50, Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>> 
>>> On Sun, Jul 4, 2021 at 10:04 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>> 
>>>> Several older compilers fail to build modern GCC because of missing
>>>> or incomplete C++11 support.
>>>> 
>>>> (although the PR mentions clang, specifically, this has also been reported
>>>> for some GCC versions within the range that should be able to bootstrap
>>>> GCC)
>>>> 
>>>> There are several possible solutions proposed in the PR, this one seems
>>>> the least invasive.
>>>> 
>>>> The header is pulled into the gcov code that builds with C, so we have to
>>>> make the CTOR conditional on C++.
>>>> 
>>>> tested on Darwin12 with xcode-6, bootstrapped on x86_64-darwin and linux.
>>>> OK for master / GCC-11?
>>> 
>>> Hmm, what is specifically built with a C compiler?  gcov.c not, I think.
>> 
>> any C compilation that includes tm.h
>> 
>> well, libgcc2 fails too on a quick check here -  but ISTR there was something in
>> libgcov and I checked with Martin that it was intentionally compiled with C compiler.
>> 
>>> Instead of commenting the CTOR, does it work to comment the whole stringop_algs
>>> type?
>> 
>> I don’t think that will work because it’s in a header that’s transitively included by tm.h
>> which is then included loads of places.
>> 
>>> Also it seems on trunk this CTOR is no more?
>> 
>> 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.
---
 gcc/config/i386/i386.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 6e0340a4b60..03d176143fe 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -73,6 +73,15 @@ struct stringop_algs
 {
   const enum stringop_alg unknown_size;
   const struct stringop_strategy {
+    /* Several older compilers delete the default constructor because of the
+       const entries (see PR100246).  Manually specifying a CTOR works around
+       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)
+      : max (_max), alg (_alg), noalign (_noalign) {}
+#endif
     const int max;
     const enum stringop_alg alg;
     int noalign;
-- 
2.24.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246]
  2021-07-06 10:17       ` Iain Sandoe
@ 2021-11-04 10:05         ` Jakub Jelinek
  2021-11-04 12:39           ` Iain Sandoe
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2021-11-04 10:05 UTC (permalink / raw)
  To: Iain Sandoe, Uros Bizjak, Jan Hubicka; +Cc: GCC Patches

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.

--- 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246]
  2021-11-04 10:05         ` [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246] Jakub Jelinek
@ 2021-11-04 12:39           ` Iain Sandoe
  2021-11-04 12:45             ` Jakub Jelinek
  0 siblings, 1 reply; 10+ messages in thread
From: Iain Sandoe @ 2021-11-04 12:39 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Uros Bizjak, Jan Hubicka, GCC Patches

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
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246]
  2021-11-04 12:39           ` Iain Sandoe
@ 2021-11-04 12:45             ` Jakub Jelinek
  2021-11-05  9:58               ` Jakub Jelinek
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2021-11-04 12:45 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: Uros Bizjak, Jan Hubicka, GCC Patches

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246]
  2021-11-04 12:45             ` Jakub Jelinek
@ 2021-11-05  9:58               ` Jakub Jelinek
  2021-11-05 10:33                 ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2021-11-05  9:58 UTC (permalink / raw)
  To: Uros Bizjak, Jan Hubicka, Richard Biener; +Cc: gcc-patches

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246]
  2021-11-05  9:58               ` Jakub Jelinek
@ 2021-11-05 10:33                 ` Richard Biener
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Biener @ 2021-11-05 10:33 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Uros Bizjak, Jan Hubicka, Richard Biener, GCC Patches

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
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-11-05 10:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-04 20:03 [PATCH] X86: Provide a CTOR for stringop_algs [PR100246] Iain Sandoe
2021-07-05 10:50 ` Richard Biener
2021-07-05 13:04   ` Iain Sandoe
2021-07-05 13:23     ` Richard Biener
2021-07-06 10:17       ` Iain Sandoe
2021-11-04 10:05         ` [PATCH] x86: Make stringop_algs::stringop_strategy ctor constexpr [PR100246] Jakub Jelinek
2021-11-04 12:39           ` Iain Sandoe
2021-11-04 12:45             ` Jakub Jelinek
2021-11-05  9:58               ` Jakub Jelinek
2021-11-05 10:33                 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).