diff mbox

transaction_safe exceptions prevent libstdc++ building for some targets

Message ID e19b9c0b-c0d8-830d-5ecf-9c6ddf63cced@somniumtech.com
State New
Headers show

Commit Message

Joe Seymour Aug. 17, 2016, 11:19 a.m. UTC
../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.

    if test $glibcxx_cv_size_t_mangling = x; then

Comments

Jonathan Wakely Aug. 17, 2016, 11:30 a.m. UTC | #1
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.
Joe Seymour Aug. 17, 2016, 11:50 a.m. UTC | #2
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
Joe Seymour Jan. 18, 2017, 7:08 p.m. UTC | #3
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?
diff mbox

Patch

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