public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* transaction_safe exceptions prevent libstdc++ building for some targets
@ 2016-08-17 11:19 Joe Seymour
  2016-08-17 11:30 ` Jonathan Wakely
  0 siblings, 1 reply; 7+ messages in thread
From: Joe Seymour @ 2016-08-17 11:19 UTC (permalink / raw)
  To: libstdc++; +Cc: gcc-patches, Torvald Riegel

../configure --target=msp430-elf --enable-languages=c,c++ && make -j4

Results in the msp430 -mlarge multilib failing to build with...

 > configure: error: Unknown underlying type for size_t
 > make[1]: *** [configure-target-libstdc++-v3] Error 1

This relates to...

 > commit 13143e139230dc4d72710a6c4c312105aeddce4f
 > Author: torvald <torvald@138bc75d-0d04-0410-961f-82ee72b054a4>
 > Date:  Fri Jan 15 22:42:41 2016 +0000
 >
 >    libstdc++: Make certain exceptions transaction_safe.

https://gcc.gnu.org/ml/libstdc++/2016-01/msg00015.html

...which iterates through a list of types at configure time, looking for one
matching __SIZE_TYPE__. The following patch (wip) allows the build to 
proceed
(to the next error), by adding __int20 to the list of types to try.

- Is this an acceptable solution?
- There's a comment above GLIBCXX_CHECK_SIZE_T_MANGLING, saying that it was
   copied from libitm. Should that be updated too, even if it's 
irrelevant for
   msp430?

Proceeding past that build error, all multilibs then fail to build with...

 > ../../../../../libstdc++-v3/src/c++11/cow-stdexcept.cc:274:3: error: 
static assertion failed: Pointers must be 32 bits or 64 bits wide
 >  static_assert(sizeof(uint64_t) == sizeof(void*)

The assert fails because msp430 has 16-bit or 20-bit pointers. The same 
error
can be reproduced for the rl78-elf target, which has 16-bit pointers.

Disabling the original changes for targets with unsupported pointer 
sizes seems
like a reasonable solution to me, but I can't see an existing mechanism 
to do
so? Do others agree?

More generally, it might be useful to have a mechanism to disable 
transactional
memory for some targets. I've observed things like register_tm_clones 
taking up
space in ARM Cortex-M binaries. Presumably, this would be achieved by 
adding an
--{enable,disable}-transactional-memory argument to configure? I'm happy to
work on a patch, if this is acceptable in principle.

Thanks,

2016-08-XX  Joe Seymour  <joe.s@somniumtech.com>

     * acinclude.m4 (GLIBCXX_CHCK_SIZE_T_MANGLING): Try __int20.
     * configure: Regenerate.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index b0f88cb..7332d3e 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -4405,9 +4405,13 @@ AC_DEFUN([GLIBCXX_CHECK_SIZE_T_MANGLING], [
                         [extern __SIZE_TYPE__ x; extern unsigned long 
long x;],
                         [glibcxx_cv_size_t_mangling=y], [
            AC_TRY_COMPILE([],
-                         [extern __SIZE_TYPE__ x; extern unsigned short 
x;],
-                         [glibcxx_cv_size_t_mangling=t],
-                         [glibcxx_cv_size_t_mangling=x])
+                         [extern __SIZE_TYPE__ x; extern unsigned long 
long x;],
+                         [glibcxx_cv_size_t_mangling=y], [
+            AC_TRY_COMPILE([],
+                           [extern __SIZE_TYPE__ x; extern __int20 
unsigned x;],
+                           [glibcxx_cv_size_t_mangling=u6uint20],
+                           [glibcxx_cv_size_t_mangling=x])
+          ])
          ])
        ])
      ])
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 41797a9..7ab84eb 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -80475,13 +80475,28 @@ else
  int
  main ()
  {
-extern __SIZE_TYPE__ x; extern unsigned short x;
+extern __SIZE_TYPE__ x; extern unsigned long long x;
    ;
    return 0;
  }
  _ACEOF
  if ac_fn_c_try_compile "$LINENO"; then :
-  glibcxx_cv_size_t_mangling=t
+  glibcxx_cv_size_t_mangling=y
+else
+
+            cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+int
+main ()
+{
+extern __SIZE_TYPE__ x; extern __int20 unsigned x;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  glibcxx_cv_size_t_mangling=u6uint20
  else
    glibcxx_cv_size_t_mangling=x
  fi
@@ -80497,6 +80512,9 @@ fi
  rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext

  fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
  { $as_echo "$as_me:${as_lineno-$LINENO}: result: 
$glibcxx_cv_size_t_mangling" >&5
  $as_echo "$glibcxx_cv_size_t_mangling" >&6; }
    if test $glibcxx_cv_size_t_mangling = x; then

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

* Re: transaction_safe exceptions prevent libstdc++ building for some targets
  2016-08-17 11:19 transaction_safe exceptions prevent libstdc++ building for some targets Joe Seymour
@ 2016-08-17 11:30 ` Jonathan Wakely
  2016-08-17 11:50   ` Joe Seymour
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2016-08-17 11:30 UTC (permalink / raw)
  To: Joe Seymour; +Cc: libstdc++, gcc-patches, Torvald Riegel

On 17/08/16 12:19 +0100, Joe Seymour wrote:
>Disabling the original changes for targets with unsupported pointer 
>sizes seems
>like a reasonable solution to me, but I can't see an existing 
>mechanism to do
>so? Do others agree?

Yes, the intention was that the transaction-safe exceptions would only
be defined where it is relatively straightforward to support them. If
they're causing build failures they can be simply disabled for that
target. Maybe the configure script should check for 32-bit or 64-bit
pointers and completely disable the TM exceptions otherwise.

>More generally, it might be useful to have a mechanism to disable 
>transactional
>memory for some targets. I've observed things like register_tm_clones 
>taking up
>space in ARM Cortex-M binaries. Presumably, this would be achieved by 
>adding an
>--{enable,disable}-transactional-memory argument to configure? I'm happy to
>work on a patch, if this is acceptable in principle.

I think that makes sense, for cases where the target can support the
TM clones but they aren't wanted.

If we add such an option then the checks for 32/64-bit pointers can go
in there, so that when the option isn't used the default depends on
the target.

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

* Re: transaction_safe exceptions prevent libstdc++ building for some targets
  2016-08-17 11:30 ` Jonathan Wakely
@ 2016-08-17 11:50   ` Joe Seymour
  2017-01-18 19:24     ` Joe Seymour
  0 siblings, 1 reply; 7+ messages in thread
From: Joe Seymour @ 2016-08-17 11:50 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches, Torvald Riegel

On 17/08/2016 12:30, Jonathan Wakely wrote:
> On 17/08/16 12:19 +0100, Joe Seymour wrote:
>> Disabling the original changes for targets with unsupported pointer sizes seems
>> like a reasonable solution to me, but I can't see an existing mechanism to do
>> so? Do others agree?
> 
> Yes, the intention was that the transaction-safe exceptions would only
> be defined where it is relatively straightforward to support them. If
> they're causing build failures they can be simply disabled for that
> target. Maybe the configure script should check for 32-bit or 64-bit
> pointers and completely disable the TM exceptions otherwise.
> 
>> More generally, it might be useful to have a mechanism to disable transactional
>> memory for some targets. I've observed things like register_tm_clones taking up
>> space in ARM Cortex-M binaries. Presumably, this would be achieved by adding an
>> --{enable,disable}-transactional-memory argument to configure? I'm happy to
>> work on a patch, if this is acceptable in principle.
> 
> I think that makes sense, for cases where the target can support the
> TM clones but they aren't wanted.
> 
> If we add such an option then the checks for 32/64-bit pointers can go
> in there, so that when the option isn't used the default depends on
> the target.
> 

I'll start working on a patch.

Thanks,

Joe

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

* Re: transaction_safe exceptions prevent libstdc++ building for some targets
  2016-08-17 11:50   ` Joe Seymour
@ 2017-01-18 19:24     ` Joe Seymour
  2017-01-18 19:29       ` DJ Delorie
  0 siblings, 1 reply; 7+ messages in thread
From: Joe Seymour @ 2017-01-18 19:24 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches, Torvald Riegel

On 17/08/2016 12:19, Joe Seymour wrote:
> fail to build with...
> 
>> ../../../../../libstdc++-v3/src/c++11/cow-stdexcept.cc:274:3: error: static assertion failed: Pointers must be 32 bits or 64 bits wide
>>  static_assert(sizeof(uint64_t) == sizeof(void*)
> 
> The assert fails because msp430 has 16-bit or 20-bit pointers. The same error
> can be reproduced for the rl78-elf target, which has 16-bit pointers.

With current master, the rl78-elf build completes successfully and the
msp430-elf build doesn't seem to trigger the assertion, but does later fail
with a presumably unrelated ICE.

> the msp430 -mlarge multilib failing to build with...
>> configure: error: Unknown underlying type for size_t
>> make[1]: *** [configure-target-libstdc++-v3] Error 1

This is still reproducible.

> Disabling the original changes for targets with unsupported pointer sizes
> seems like a reasonable solution to me [...]

> Presumably, this would be achieved by adding an
> --{enable,disable}-transactional-memory argument to configure

I have an initial/partial patch that adds a new configure argument:
  --enable-transactional-memory=yes|no|auto

Having said that, since I now only see issues with msp430-elf, I wonder if the
configure argument might be excessive, and if my original patch to
GLIBCXX_CHECK_SIZE_T_MANGLING might be more appropriate?

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

* Re: transaction_safe exceptions prevent libstdc++ building for some targets
  2017-01-18 19:24     ` Joe Seymour
@ 2017-01-18 19:29       ` DJ Delorie
  2017-01-19 12:07         ` Joe Seymour
  0 siblings, 1 reply; 7+ messages in thread
From: DJ Delorie @ 2017-01-18 19:29 UTC (permalink / raw)
  To: Joe Seymour; +Cc: libstdc++, gcc-patches

Joe Seymour <joe.s@somniumtech.com> writes:
>> the msp430 -mlarge multilib failing to build with...
>>> configure: error: Unknown underlying type for size_t
>>> make[1]: *** [configure-target-libstdc++-v3] Error 1
>
> This is still reproducible.

FYI the underlying type is uint20_t

I think I've complained that libstdc++ has a hard-coded list, rather
than using a configure-time check, in the past...

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

* Re: transaction_safe exceptions prevent libstdc++ building for some targets
  2017-01-18 19:29       ` DJ Delorie
@ 2017-01-19 12:07         ` Joe Seymour
  2017-01-20 18:14           ` Jonathan Wakely
  0 siblings, 1 reply; 7+ messages in thread
From: Joe Seymour @ 2017-01-19 12:07 UTC (permalink / raw)
  To: DJ Delorie; +Cc: libstdc++, gcc-patches

On 18/01/2017 19:24, DJ Delorie wrote:
> Joe Seymour <joe.s@somniumtech.com> writes:
>>> the msp430 -mlarge multilib failing to build with...
>>>> configure: error: Unknown underlying type for size_t
>>>> make[1]: *** [configure-target-libstdc++-v3] Error 1
>>
>> This is still reproducible.
> 
> FYI the underlying type is uint20_t
> 
> I think I've complained that libstdc++ has a hard-coded list, rather
> than using a configure-time check, in the past...
> 

Thanks!

Here's the patch I'm proposing. I've tested it as follows:

- msp430-elf no longer encounters the error when configuring libstdc++-v3. Note
  that libstdc++-v3 doesn't build due to an ICE though.

- Configuring libstdc++-v3 for x86_64-unknown-linux-gnu produces:
  include/bits/c++config.h:#define _GLIBCXX_MANGLE_SIZE_T m
  Both with and without this patch.

- Configuring libstdc++-v3 for i686-unknown-linux-gnu produces:
  include/bits/c++config.h:#define _GLIBCXX_MANGLE_SIZE_T j
  Both with and without this patch.

If it's acceptable I would appreciate it if someone would commit it on my
behalf.

Thanks,

2017-01-19  Joe Seymour  <joe.s@somniumtech.com>

	libstdc++-v3/
	* acinclude.m4 (GLIBCXX_CHECK_SIZE_T_MANGLING): Support uint20_t.
	* configure: Regenerate.
---
 libstdc++-v3/acinclude.m4 |    8 ++++++--
 libstdc++-v3/configure    |   18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 4e04cce..d9859aa 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -4460,8 +4460,12 @@ AC_DEFUN([GLIBCXX_CHECK_SIZE_T_MANGLING], [
                        [glibcxx_cv_size_t_mangling=y], [
           AC_TRY_COMPILE([],
                          [extern __SIZE_TYPE__ x; extern unsigned short x;],
-                         [glibcxx_cv_size_t_mangling=t],
-                         [glibcxx_cv_size_t_mangling=x])
+                         [glibcxx_cv_size_t_mangling=t], [
+            AC_TRY_COMPILE([],
+                           [extern __SIZE_TYPE__ x; extern __int20 unsigned x;],
+                           [glibcxx_cv_size_t_mangling=u6uint20],
+                           [glibcxx_cv_size_t_mangling=x])
+          ])
         ])
       ])
     ])
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 219a6a3..9bb9862 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -80707,6 +80707,21 @@ _ACEOF
 if ac_fn_c_try_compile "$LINENO"; then :
   glibcxx_cv_size_t_mangling=t
 else
+
+            cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+int
+main ()
+{
+extern __SIZE_TYPE__ x; extern __int20 unsigned x;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  glibcxx_cv_size_t_mangling=u6uint20
+else
   glibcxx_cv_size_t_mangling=x
 fi
 rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
@@ -80721,6 +80736,9 @@ fi
 rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 
 fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $glibcxx_cv_size_t_mangling" >&5
 $as_echo "$glibcxx_cv_size_t_mangling" >&6; }
   if test $glibcxx_cv_size_t_mangling = x; then
-- 
1.7.1

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

* Re: transaction_safe exceptions prevent libstdc++ building for some targets
  2017-01-19 12:07         ` Joe Seymour
@ 2017-01-20 18:14           ` Jonathan Wakely
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2017-01-20 18:14 UTC (permalink / raw)
  To: Joe Seymour; +Cc: DJ Delorie, libstdc++, gcc-patches

On 19/01/17 12:05 +0000, Joe Seymour wrote:
>Here's the patch I'm proposing. I've tested it as follows:
>
>- msp430-elf no longer encounters the error when configuring libstdc++-v3. Note
>  that libstdc++-v3 doesn't build due to an ICE though.
>
>- Configuring libstdc++-v3 for x86_64-unknown-linux-gnu produces:
>  include/bits/c++config.h:#define _GLIBCXX_MANGLE_SIZE_T m
>  Both with and without this patch.
>
>- Configuring libstdc++-v3 for i686-unknown-linux-gnu produces:
>  include/bits/c++config.h:#define _GLIBCXX_MANGLE_SIZE_T j
>  Both with and without this patch.
>
>If it's acceptable I would appreciate it if someone would commit it on my
>behalf.

Done - it's committed to trunk now. Thanks for the patch.

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

end of thread, other threads:[~2017-01-20 17:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-17 11:19 transaction_safe exceptions prevent libstdc++ building for some targets Joe Seymour
2016-08-17 11:30 ` Jonathan Wakely
2016-08-17 11:50   ` Joe Seymour
2017-01-18 19:24     ` Joe Seymour
2017-01-18 19:29       ` DJ Delorie
2017-01-19 12:07         ` Joe Seymour
2017-01-20 18:14           ` Jonathan Wakely

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