C++11 code in the gcc 10 branch

C++11 code in the gcc 10 branch

[FX]

I’m trying to bootstrap a GCC 10 compiler on macOS with clang, and I am getting errors in stage 1, because there is C++11 code that is rejected by clang (because the bootstrap involves compiling stage 1 with -std=gnu++98, online on master (see top-level configure.ac). These errors are not seen, I believe, when GCC is the bootstrap compiler, because GCC will issue a warning instead of an error (as clang does).

One place with such issue is in aarch64-builtins.c, which contains a C++11 constructor. I can fix it with this:

diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index cba596765..184e9095d 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -1225,8 +1225,9 @@ aarch64_init_memtag_builtins (void)
     = aarch64_general_add_builtin ("__builtin_aarch64_memtag_"#N, \
                                   T, AARCH64_MEMTAG_BUILTIN_##F); \
   aarch64_memtag_builtin_data[AARCH64_MEMTAG_BUILTIN_##F - \
-                             AARCH64_MEMTAG_BUILTIN_START - 1] = \
-                               {T, CODE_FOR_##I};
+                             AARCH64_MEMTAG_BUILTIN_START - 1].ftype = T; \
+  aarch64_memtag_builtin_data[AARCH64_MEMTAG_BUILTIN_##F - \
+                             AARCH64_MEMTAG_BUILTIN_START - 1].icode = CODE_FOR_##I;
 
   fntype = build_function_type_list (ptr_type_node, ptr_type_node,
                                     uint64_type_node, NULL);


but later I am getting further errors:

../../gcc/gcc/config/darwin.c:1357:16: error: no viable conversion from 'poly_uint16' (aka 'poly_int<2, unsigned short>') to 'unsigned int'
  unsigned int modesize = GET_MODE_BITSIZE (mode);
               ^          ~~~~~~~~~~~~~~~~~~~~~~~
../../gcc/gcc/config/darwin.c:1752:28: error: invalid operands to binary expression ('poly_uint16' (aka 'poly_int<2, unsigned short>') and 'int')
  if (GET_MODE_SIZE (mode) == 8
      ~~~~~~~~~~~~~~~~~~~~ ^  ~
../../gcc/gcc/wide-int.h:3289:19: note: candidate template ignored: substitution failure [with T1 = poly_int<2, unsigned short>, T2 = int]: implicit instantiation of undefined template
      'wi::int_traits<poly_int<2, unsigned short> >'
BINARY_PREDICATE (operator ==, eq_p)
                  ^
../../gcc/gcc/wide-int.h:3266:3: note: expanded from macro 'BINARY_PREDICATE'
  OP (const T1 &x, const T2 &y) \
  ^
../../gcc/gcc/config/darwin.c:1757:33: error: invalid operands to binary expression ('poly_uint16' (aka 'poly_int<2, unsigned short>') and 'int')
  else if (GET_MODE_SIZE (mode) == 4
           ~~~~~~~~~~~~~~~~~~~~ ^  ~
../../gcc/gcc/wide-int.h:3289:19: note: candidate template ignored: substitution failure [with T1 = poly_int<2, unsigned short>, T2 = int]: implicit instantiation of undefined template
      'wi::int_traits<poly_int<2, unsigned short> >'
BINARY_PREDICATE (operator ==, eq_p)
                  ^
../../gcc/gcc/wide-int.h:3266:3: note: expanded from macro 'BINARY_PREDICATE'
  OP (const T1 &x, const T2 &y) \
  ^
../../gcc/gcc/config/darwin.c:1764:29: error: invalid operands to binary expression ('poly_uint16' (aka 'poly_int<2, unsigned short>') and 'int')
           && GET_MODE_SIZE (mode) == 16
              ~~~~~~~~~~~~~~~~~~~~ ^  ~~
../../gcc/gcc/wide-int.h:3289:19: note: candidate template ignored: substitution failure [with T1 = poly_int<2, unsigned short>, T2 = int]: implicit instantiation of undefined template
      'wi::int_traits<poly_int<2, unsigned short> >'
BINARY_PREDICATE (operator ==, eq_p)
                  ^
../../gcc/gcc/wide-int.h:3266:3: note: expanded from macro 'BINARY_PREDICATE'
  OP (const T1 &x, const T2 &y) \
  ^


I would welcome:

1. confirmation that the C++11 code in aarch64-builtins.c is indeed a bug, and that a patch for it would be welcome
2. guidance about how to fix that next issue


Thanks,
FX

Re: C++11 code in the gcc 10 branch

[Iain Sandoe]

FX via Gcc <gcc@gcc.gnu.org> wrote:

<SNIP>

> but later I am getting further errors:
>
> ../../gcc/gcc/config/darwin.c:1357:16: error: no viable conversion from  
> 'poly_uint16' (aka 'poly_int<2, unsigned short>') to 'unsigned int'
>  unsigned int modesize = GET_MODE_BITSIZE (mode);
>               ^          ~~~~~~~~~~~~~~~~~~~~~~~
> ../../gcc/gcc/config/darwin.c:1752:28: error: invalid operands to binary  
> expression ('poly_uint16' (aka 'poly_int<2, unsigned short>') and 'int')
>  if (GET_MODE_SIZE (mode) == 8


> 1. confirmation that the C++11 code in aarch64-builtins.c is indeed a  
> bug, and that a patch for it would be welcome
> 2. guidance about how to fix that next issue

You are missing a patch from master that converted darwin.c to use the  
“proper” handling of poly_int16 instead of the workaround.

7ddee9cd99b, I think is the one.

Iain

Re: C++11 code in the gcc 10 branch

[Richard Sandiford]

FX via Gcc <gcc@gcc.gnu.org> writes:
> I’m trying to bootstrap a GCC 10 compiler on macOS with clang, and I am getting errors in stage 1, because there is C++11 code that is rejected by clang (because the bootstrap involves compiling stage 1 with -std=gnu++98, online on master (see top-level configure.ac). These errors are not seen, I believe, when GCC is the bootstrap compiler, because GCC will issue a warning instead of an error (as clang does).
>
> One place with such issue is in aarch64-builtins.c, which contains a C++11 constructor. I can fix it with this:
>
> diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
> index cba596765..184e9095d 100644
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -1225,8 +1225,9 @@ aarch64_init_memtag_builtins (void)
>      = aarch64_general_add_builtin ("__builtin_aarch64_memtag_"#N, \
>                                    T, AARCH64_MEMTAG_BUILTIN_##F); \
>    aarch64_memtag_builtin_data[AARCH64_MEMTAG_BUILTIN_##F - \
> -                             AARCH64_MEMTAG_BUILTIN_START - 1] = \
> -                               {T, CODE_FOR_##I};
> +                             AARCH64_MEMTAG_BUILTIN_START - 1].ftype = T; \
> +  aarch64_memtag_builtin_data[AARCH64_MEMTAG_BUILTIN_##F - \
> +                             AARCH64_MEMTAG_BUILTIN_START - 1].icode = CODE_FOR_##I;
>  
>    fntype = build_function_type_list (ptr_type_node, ptr_type_node,
>                                      uint64_type_node, NULL);
>
> […stuff that Iain has already answered…]
>
> I would welcome:
>
> 1. confirmation that the C++11 code in aarch64-builtins.c is indeed a bug, and that a patch for it would be welcome

Yeah, it's definitely a bug, thanks for catching it.  The patch above
is OK.

Thanks,
Richard

Re: C++11 code in the gcc 10 branch

[Gerald Pfeifer]

On Mon, 21 Dec 2020, FX via Gcc wrote:
> I’m trying to bootstrap a GCC 10 compiler on macOS with clang, and I am 
> getting errors in stage 1, because there is C++11 code that is rejected 
> by clang (because the bootstrap involves compiling stage 1 with 
> -std=gnu++98, online on master (see top-level configure.ac).

I got reports from users, too (for FreeBSD with clang as system 
compiler), and have them test this patch which looks fine so far.

> These errors are not seen, I believe, when GCC is the bootstrap 
> compiler, because GCC will issue a warning instead of an error (as 
> clang does).
> 
> One place with such issue is in aarch64-builtins.c, which contains a 
> C++11 constructor. I can fix it with this:

When are you going to apply your fix that Richard S. approved on the
21st?

Gerald

Re: C++11 code in the gcc 10 branch

[FX]

> When are you going to apply your fix that Richard S. approved on the
> 21st?

When I remember how to set up gcc’s git with write access, and remember how the new ChangeLog entries work. The times where I was a regular contributor were the CVS and SVN times.

I also wanted to ask approval to commit this diff below, fixing aarch64_get_extension_string_for_isa_flags()’s prototype to align it with the actual function definition:

diff --git a/gcc/config/aarch64/driver-aarch64.c b/gcc/config/aarch64/driver-aarch64.c
index 8840a2d9486c..d99834c99896 100644
--- a/gcc/config/aarch64/driver-aarch64.c
+++ b/gcc/config/aarch64/driver-aarch64.c
@@ -27,8 +27,7 @@
 #include "tm.h"
 
 /* Defined in common/config/aarch64/aarch64-common.c.  */
-std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
-							unsigned long);
+std::string aarch64_get_extension_string_for_isa_flags (uint64_t, uint64_t);
 
 struct aarch64_arch_extension
 {


Although I admit that’s almost trivial (and it breaks build on aarch64-darwin), I’d prefer to be sure and ask. Then I’ll commit the two patches, if you think that’s OK.

FX

Re: C++11 code in the gcc 10 branch

[Iain Sandoe]

FX <fxcoudert@gmail.com> wrote:

>> When are you going to apply your fix that Richard S. approved on the
>> 21st?
>
> When I remember how to set up gcc’s git with write access, and remember  
> how the new ChangeLog entries work. The times where I was a regular  
> contributor were the CVS and SVN times.
>
> I also wanted to ask approval to commit this diff below, fixing  
> aarch64_get_extension_string_for_isa_flags()’s prototype to align it with  
> the actual function definition:
>
> diff --git a/gcc/config/aarch64/driver-aarch64.c  
> b/gcc/config/aarch64/driver-aarch64.c
> index 8840a2d9486c..d99834c99896 100644
> --- a/gcc/config/aarch64/driver-aarch64.c
> +++ b/gcc/config/aarch64/driver-aarch64.c
> @@ -27,8 +27,7 @@
> #include "tm.h"
>
> /* Defined in common/config/aarch64/aarch64-common.c.  */
> -std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
> -							unsigned long);
> +std::string aarch64_get_extension_string_for_isa_flags (uint64_t,  
> uint64_t);
>
> struct aarch64_arch_extension
> {
>
>
> Although I admit that’s almost trivial (and it breaks build on  
> aarch64-darwin), I’d prefer to be sure and ask. Then I’ll commit the two  
> patches, if you think that’s OK.

If Richard approves the second patch (and you’re stuck for time) - then  
send me the patch(es) as attachments with the commit credits you want, and  
I can apply them for you.

(thanks for the fixes)

cheers & happy new year all,

Iain

Re: C++11 code in the gcc 10 branch

[FX]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

> If Richard approves the second patch (and you’re stuck for time) - then send me the patch(es) as attachments with the commit credits you want, and I can apply them for you.

Both patches only needed on gcc-10, if you can commit that’s great, many thanks.

For credits in GCC I’m known as Francois-Xavier Coudert <fxcoudert@gcc.gnu.org>
(and for the record, I do have a copyright assignment in place)

Happy new year to all,
FX


[-- Attachment #2: p1.txt --]
[-- Type: text/plain, Size: 793 bytes --]

commit 96a9d0950453ca571b3999c2f4d4168da9d770f0
Author: Francois-Xavier Coudert <fxcoudert@gmail.com>
Date:   2020-12-21 22:06:59 +0100

    Fix prototype for aarch64_get_extension_string_for_isa_flags
    
    Patch should be backported in gcc-10 branch

diff --git a/gcc/config/aarch64/driver-aarch64.c b/gcc/config/aarch64/driver-aarch64.c
index 8840a2d9486..d99834c9989 100644
--- a/gcc/config/aarch64/driver-aarch64.c
+++ b/gcc/config/aarch64/driver-aarch64.c
@@ -27,8 +27,7 @@
 #include "tm.h"
 
 /* Defined in common/config/aarch64/aarch64-common.c.  */
-std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
-							unsigned long);
+std::string aarch64_get_extension_string_for_isa_flags (uint64_t, uint64_t);
 
 struct aarch64_arch_extension
 {

[-- Attachment #3: p2.txt --]
[-- Type: text/plain, Size: 1054 bytes --]

commit 3f1b98679eecdaf81ba5702a8de65bcb6b4ab25f
Author: Francois-Xavier Coudert <fxcoudert@gmail.com>
Date:   2020-12-21 22:06:15 +0100

    Remove C++11 constructor
    
    Patch should be backported in gcc-10 branch

diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index cba596765ac..184e9095d51 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -1225,8 +1225,9 @@ aarch64_init_memtag_builtins (void)
     = aarch64_general_add_builtin ("__builtin_aarch64_memtag_"#N, \
 				   T, AARCH64_MEMTAG_BUILTIN_##F); \
   aarch64_memtag_builtin_data[AARCH64_MEMTAG_BUILTIN_##F - \
-			      AARCH64_MEMTAG_BUILTIN_START - 1] = \
-				{T, CODE_FOR_##I};
+			      AARCH64_MEMTAG_BUILTIN_START - 1].ftype = T; \
+  aarch64_memtag_builtin_data[AARCH64_MEMTAG_BUILTIN_##F - \
+			      AARCH64_MEMTAG_BUILTIN_START - 1].icode = CODE_FOR_##I;
 
   fntype = build_function_type_list (ptr_type_node, ptr_type_node,
 				     uint64_type_node, NULL);

Re: C++11 code in the gcc 10 branch

[Iain Sandoe]

FX <fxcoudert@gmail.com> wrote:

>> If Richard approves the second patch (and you’re stuck for time) - then  
>> send me the patch(es) as attachments with the commit credits you want,  
>> and I can apply them for you.
>
> Both patches only needed on gcc-10, if you can commit that’s great, many  
> thanks.

bootstrapped / smoke tested on aarch64-linux-gnu and x86_64-darwin,
pushed as r10-9187 and r10-9188.
Iain

Re: C++11 code in the gcc 10 branch

[Richard Sandiford]

FX <fxcoudert@gmail.com> writes:
>> When are you going to apply your fix that Richard S. approved on the
>> 21st?
>
> When I remember how to set up gcc’s git with write access, and remember how the new ChangeLog entries work. The times where I was a regular contributor were the CVS and SVN times.
>
> I also wanted to ask approval to commit this diff below, fixing aarch64_get_extension_string_for_isa_flags()’s prototype to align it with the actual function definition:
>
> diff --git a/gcc/config/aarch64/driver-aarch64.c b/gcc/config/aarch64/driver-aarch64.c
> index 8840a2d9486c..d99834c99896 100644
> --- a/gcc/config/aarch64/driver-aarch64.c
> +++ b/gcc/config/aarch64/driver-aarch64.c
> @@ -27,8 +27,7 @@
>  #include "tm.h"
>  
>  /* Defined in common/config/aarch64/aarch64-common.c.  */
> -std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
> -							unsigned long);
> +std::string aarch64_get_extension_string_for_isa_flags (uint64_t, uint64_t);
>  
>  struct aarch64_arch_extension
>  {

Yeah, that's OK everywhere it's needed, thanks.  (The other patch
was only needed on release branches.)

Richard

Re: C++11 code in the gcc 10 branch

[Andreas Schwab]

On Dez 31 2020, FX via Gcc wrote:

> I also wanted to ask approval to commit this diff below, fixing aarch64_get_extension_string_for_isa_flags()’s prototype to align it with the actual function definition:
>
> diff --git a/gcc/config/aarch64/driver-aarch64.c b/gcc/config/aarch64/driver-aarch64.c
> index 8840a2d9486c..d99834c99896 100644
> --- a/gcc/config/aarch64/driver-aarch64.c
> +++ b/gcc/config/aarch64/driver-aarch64.c
> @@ -27,8 +27,7 @@
>  #include "tm.h"
>  
>  /* Defined in common/config/aarch64/aarch64-common.c.  */
> -std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
> -							unsigned long);
> +std::string aarch64_get_extension_string_for_isa_flags (uint64_t, uint64_t);
>  
>  struct aarch64_arch_extension
>  {

See commit cb5ecbc49b7.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."