diff mbox series

PR libstdc++/86846 Alternative to pointer-width atomics

Message ID 20180814131339.GA9685@redhat.com
State New
Headers show
Series PR libstdc++/86846 Alternative to pointer-width atomics | expand

Commit Message

Jonathan Wakely Aug. 14, 2018, 1:13 p.m. UTC 
Define a class using std::mutex for when std::atomic<memory_resource*>
cannot be used to implement the default memory resource.

When std::mutex constructor is not constexpr the constant_init trick
won't work, so just define a global and use init_priority for it. The
compiler warns about using reserved priority, so put the definition in a
header file using #pragma GCC system_header to suppress the warning.

	PR libstdc++/86846
	* src/c++17/default_resource.h: New file, defining default_res.
	* src/c++17/memory_resource.cc [ATOMIC_POINTER_LOCK_FREE != 2]
	(atomic_mem_res): Define alternative for atomic<memory_resource*>
	using a mutex instead of atomics.

Tested x86_64-linux, committed to trunk.
commit 81de951109506b684d079f0fbc7aebae2c228ba4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Aug 13 12:34:56 2018 +0100

    PR libstdc++/86846 Alternative to pointer-width atomics
    
    Define a class using std::mutex for when std::atomic<memory_resource*>
    cannot be used to implement the default memory resource.
    
    When std::mutex constructor is not constexpr the constant_init trick
    won't work, so just define a global and use init_priority for it. The
    compiler warns about using reserved priority, so put the definition in a
    header file using #pragma GCC system_header to suppress the warning.
    
            PR libstdc++/86846
            * src/c++17/default_resource.h: New file, defining default_res.
            * src/c++17/memory_resource.cc [ATOMIC_POINTER_LOCK_FREE != 2]
            (atomic_mem_res): Define alternative for atomic<memory_resource*>
            using a mutex instead of atomics.

Comments

Szabolcs Nagy Aug. 15, 2018, 9:40 a.m. UTC | #1 
On 14/08/18 14:13, Jonathan Wakely wrote:
> Define a class using std::mutex for when std::atomic<memory_resource*>
> cannot be used to implement the default memory resource.
> 
> When std::mutex constructor is not constexpr the constant_init trick
> won't work, so just define a global and use init_priority for it. The
> compiler warns about using reserved priority, so put the definition in a
> header file using #pragma GCC system_header to suppress the warning.
> 
>      PR libstdc++/86846
>      * src/c++17/default_resource.h: New file, defining default_res.
>      * src/c++17/memory_resource.cc [ATOMIC_POINTER_LOCK_FREE != 2]
>      (atomic_mem_res): Define alternative for atomic<memory_resource*>
>      using a mutex instead of atomics.
> 
> Tested x86_64-linux, committed to trunk.
> 

build fails with arm-none-eabi target:

Making all in c++17
make[4]: Entering directory '/B/arm-none-eabi/libstdc++-v3/src/c++17'
/bin/sh ../../libtool --tag CXX --tag disable-shared   --mode=compile /B/./gcc/xgcc -shared-libgcc -B/B/./gcc -nostdinc++ 
-L/B/arm-none-eabi/libstdc++-v3/src -L/B/arm-none-eabi/libstdc++-v3/src/.libs -L/B/arm-none-eabi/libstdc++-v3/libsupc++/.libs 
-B/P/arm-none-eabi/bin/ -B/P/arm-none-eabi/lib/ -isystem /P/arm-none-eabi/include -isystem /P/arm-none-eabi/sys-include 
-I/S/libstdc++-v3/../libgcc -I/B/arm-none-eabi/libstdc++-v3/include/arm-none-eabi -I/B/arm-none-eabi/libstdc++-v3/include 
-I/S/libstdc++-v3/libsupc++   -std=gnu++17   -fno-implicit-templates  -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi=2 
-fdiagnostics-show-location=once   -ffunction-sections -fdata-sections  -frandom-seed=memory_resource.lo -g -ffunction-sections -fdata-sections 
-O2  -c -o memory_resource.lo /S/libstdc++-v3/src/c++17/memory_resource.cc
libtool: compile:  /B/./gcc/xgcc -shared-libgcc -B/B/./gcc -nostdinc++ -L/B/arm-none-eabi/libstdc++-v3/src 
-L/B/arm-none-eabi/libstdc++-v3/src/.libs -L/B/arm-none-eabi/libstdc++-v3/libsupc++/.libs -B/P/arm-none-eabi/bin/ -B/P/arm-none-eabi/lib/ 
-isystem /P/arm-none-eabi/include -isystem /P/arm-none-eabi/sys-include -I/S/libstdc++-v3/../libgcc 
-I/B/arm-none-eabi/libstdc++-v3/include/arm-none-eabi -I/B/arm-none-eabi/libstdc++-v3/include -I/S/libstdc++-v3/libsupc++ -std=gnu++17 
-fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi=2 -fdiagnostics-show-location=once -ffunction-sections -fdata-sections 
-frandom-seed=memory_resource.lo -g -ffunction-sections -fdata-sections -O2 -c /S/libstdc++-v3/src/c++17/memory_resource.cc -o memory_resource.o
/S/libstdc++-v3/src/c++17/memory_resource.cc:102:7: error: 'mutex' does not name a type
102 |       mutex mx;
     |       ^~~~~
/S/libstdc++-v3/src/c++17/memory_resource.cc: In member function 'std::pmr::memory_resource* std::pmr::{anonymous}::atomic_mem_res::load()':
/S/libstdc++-v3/src/c++17/memory_resource.cc:107:13: error: 'mutex' was not declared in this scope
107 |  lock_guard<mutex> lock(mx);
     |             ^~~~~
/S/libstdc++-v3/src/c++17/memory_resource.cc:107:18: error: template argument 1 is invalid
107 |  lock_guard<mutex> lock(mx);
     |                  ^
/S/libstdc++-v3/src/c++17/memory_resource.cc:107:25: error: 'mx' was not declared in this scope
107 |  lock_guard<mutex> lock(mx);
     |                         ^~
/S/libstdc++-v3/src/c++17/memory_resource.cc:107:25: note: suggested alternative: 'max'
107 |  lock_guard<mutex> lock(mx);
     |                         ^~
     |                         max
/S/libstdc++-v3/src/c++17/memory_resource.cc:107:20: warning: unused variable 'lock' [-Wunused-variable]
107 |  lock_guard<mutex> lock(mx);
     |                    ^~~~
/S/libstdc++-v3/src/c++17/memory_resource.cc: In member function 'std::pmr::memory_resource* 
std::pmr::{anonymous}::atomic_mem_res::exchange(std::pmr::memory_resource*)':
/S/libstdc++-v3/src/c++17/memory_resource.cc:113:13: error: 'mutex' was not declared in this scope
113 |  lock_guard<mutex> lock(mx);
     |             ^~~~~
/S/libstdc++-v3/src/c++17/memory_resource.cc:113:18: error: template argument 1 is invalid
113 |  lock_guard<mutex> lock(mx);
     |                  ^
/S/libstdc++-v3/src/c++17/memory_resource.cc:113:25: error: 'mx' was not declared in this scope
113 |  lock_guard<mutex> lock(mx);
     |                         ^~
/S/libstdc++-v3/src/c++17/memory_resource.cc:113:25: note: suggested alternative: 'max'
113 |  lock_guard<mutex> lock(mx);
     |                         ^~
     |                         max
/S/libstdc++-v3/src/c++17/memory_resource.cc:113:20: warning: unused variable 'lock' [-Wunused-variable]
113 |  lock_guard<mutex> lock(mx);
     |                    ^~~~
make[4]: *** [Makefile:471: memory_resource.lo] Error 1
make[4]: Leaving directory '/B/arm-none-eabi/libstdc++-v3/src/c++17'
make[3]: *** [Makefile:643: all-recursive] Error 1
make[3]: Leaving directory '/B/arm-none-eabi/libstdc++-v3/src'
make[2]: *** [Makefile:510: all-recursive] Error 1
make[2]: Leaving directory '/B/arm-none-eabi/libstdc++-v3'
make[1]: *** [Makefile:417: all] Error 2
make[1]: Leaving directory '/B/arm-none-eabi/libstdc++-v3'
make: *** [Makefile:10864: all-target-libstdc++-v3] Error 2
make: *** Waiting for unfinished jobs....
Jonathan Wakely Aug. 15, 2018, 9:59 a.m. UTC | #2 
On 15/08/18 10:40 +0100, Szabolcs Nagy wrote:
>On 14/08/18 14:13, Jonathan Wakely wrote:
>>Define a class using std::mutex for when std::atomic<memory_resource*>
>>cannot be used to implement the default memory resource.
>>
>>When std::mutex constructor is not constexpr the constant_init trick
>>won't work, so just define a global and use init_priority for it. The
>>compiler warns about using reserved priority, so put the definition in a
>>header file using #pragma GCC system_header to suppress the warning.
>>
>>     PR libstdc++/86846
>>     * src/c++17/default_resource.h: New file, defining default_res.
>>     * src/c++17/memory_resource.cc [ATOMIC_POINTER_LOCK_FREE != 2]
>>     (atomic_mem_res): Define alternative for atomic<memory_resource*>
>>     using a mutex instead of atomics.
>>
>>Tested x86_64-linux, committed to trunk.
>>
>
>build fails with arm-none-eabi target:
>
>Making all in c++17
>make[4]: Entering directory '/B/arm-none-eabi/libstdc++-v3/src/c++17'
>/bin/sh ../../libtool --tag CXX --tag disable-shared   --mode=compile 
>/B/./gcc/xgcc -shared-libgcc -B/B/./gcc -nostdinc++ 
>-L/B/arm-none-eabi/libstdc++-v3/src 
>-L/B/arm-none-eabi/libstdc++-v3/src/.libs 
>-L/B/arm-none-eabi/libstdc++-v3/libsupc++/.libs 
>-B/P/arm-none-eabi/bin/ -B/P/arm-none-eabi/lib/ -isystem 
>/P/arm-none-eabi/include -isystem /P/arm-none-eabi/sys-include 
>-I/S/libstdc++-v3/../libgcc 
>-I/B/arm-none-eabi/libstdc++-v3/include/arm-none-eabi 
>-I/B/arm-none-eabi/libstdc++-v3/include -I/S/libstdc++-v3/libsupc++   
>-std=gnu++17   -fno-implicit-templates  -Wall -Wextra -Wwrite-strings 
>-Wcast-qual -Wabi=2 -fdiagnostics-show-location=once   
>-ffunction-sections -fdata-sections  -frandom-seed=memory_resource.lo 
>-g -ffunction-sections -fdata-sections -O2  -c -o memory_resource.lo 
>/S/libstdc++-v3/src/c++17/memory_resource.cc
>libtool: compile:  /B/./gcc/xgcc -shared-libgcc -B/B/./gcc -nostdinc++ 
>-L/B/arm-none-eabi/libstdc++-v3/src 
>-L/B/arm-none-eabi/libstdc++-v3/src/.libs 
>-L/B/arm-none-eabi/libstdc++-v3/libsupc++/.libs 
>-B/P/arm-none-eabi/bin/ -B/P/arm-none-eabi/lib/ -isystem 
>/P/arm-none-eabi/include -isystem /P/arm-none-eabi/sys-include 
>-I/S/libstdc++-v3/../libgcc 
>-I/B/arm-none-eabi/libstdc++-v3/include/arm-none-eabi 
>-I/B/arm-none-eabi/libstdc++-v3/include -I/S/libstdc++-v3/libsupc++ 
>-std=gnu++17 -fno-implicit-templates -Wall -Wextra -Wwrite-strings 
>-Wcast-qual -Wabi=2 -fdiagnostics-show-location=once 
>-ffunction-sections -fdata-sections -frandom-seed=memory_resource.lo 
>-g -ffunction-sections -fdata-sections -O2 -c 
>/S/libstdc++-v3/src/c++17/memory_resource.cc -o memory_resource.o
>/S/libstdc++-v3/src/c++17/memory_resource.cc:102:7: error: 'mutex' does not name a type
>102 |       mutex mx;

Sorry about that, it should be fixed at r263554 by the commit of this
patch.
commit 21cebbbedfbbe8fc5c4b1ebc59bef1b8f090bc6b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Aug 15 10:55:31 2018 +0100

    Fix single-threaded build for targets without atomics
    
            * src/c++17/memory_resource.cc [!_GLIBCXX_HAS_GTHREADS]
            (atomic_mem_res): Add unsynchronized definition for single-threaded.

diff --git a/libstdc++-v3/src/c++17/memory_resource.cc b/libstdc++-v3/src/c++17/memory_resource.cc
index bd8f32d931e..aa82813e645 100644
--- a/libstdc++-v3/src/c++17/memory_resource.cc
+++ b/libstdc++-v3/src/c++17/memory_resource.cc
@@ -88,7 +88,7 @@ namespace pmr
 #if ATOMIC_POINTER_LOCK_FREE == 2
     using atomic_mem_res = atomic<memory_resource*>;
 # define _GLIBCXX_ATOMIC_MEM_RES_CAN_BE_CONSTANT_INITIALIZED
-#else
+#elif defined(_GLIBCXX_HAS_GTHREADS)
     // Can't use pointer-width atomics, define a type using a mutex instead:
     struct atomic_mem_res
     {
@@ -114,6 +114,26 @@ namespace pmr
 	return std::exchange(val, r);
       }
     };
+#else
+# define _GLIBCXX_ATOMIC_MEM_RES_CAN_BE_CONSTANT_INITIALIZED
+    // Single-threaded, no need for synchronization
+    struct atomic_mem_res
+    {
+      constexpr
+      atomic_mem_res(memory_resource* r) : val(r) { }
+
+      memory_resource* val;
+
+      memory_resource* load() const
+      {
+	return val;
+      }
+
+      memory_resource* exchange(memory_resource* r)
+      {
+	return std::exchange(val, r);
+      }
+    };
 #endif // ATOMIC_POINTER_LOCK_FREE == 2
 
 #ifdef _GLIBCXX_ATOMIC_MEM_RES_CAN_BE_CONSTANT_INITIALIZED
diff mbox series

Patch

diff --git a/libstdc++-v3/src/c++17/default_resource.h b/libstdc++-v3/src/c++17/default_resource.h
new file mode 100644
index 00000000000..522cee13b90
--- /dev/null
+++ b/libstdc++-v3/src/c++17/default_resource.h
@@ -0,0 +1,11 @@ 
+// This is only in a header so we can use the system_header pragma,
+// to suppress the warning caused by using a reserved init_priority.
+#pragma GCC system_header
+
+#if ATOMIC_POINTER_LOCK_FREE == 2 || defined(__GTHREAD_MUTEX_INIT)
+# error "This file should not be included for this build"
+#endif
+
+struct {
+  atomic_mem_res obj = &newdel_res.obj;
+} default_res __attribute__ ((init_priority (100)));
diff --git a/libstdc++-v3/src/c++17/memory_resource.cc b/libstdc++-v3/src/c++17/memory_resource.cc
index c3ae2b69f71..bd8f32d931e 100644
--- a/libstdc++-v3/src/c++17/memory_resource.cc
+++ b/libstdc++-v3/src/c++17/memory_resource.cc
@@ -25,6 +25,10 @@ 
 #include <memory_resource>
 #include <atomic>
 #include <new>
+#if ATOMIC_POINTER_LOCK_FREE != 2
+# include <bits/std_mutex.h>	// std::mutex, std::lock_guard
+# include <bits/move.h>		// std::exchange
+#endif
 
 namespace std _GLIBCXX_VISIBILITY(default)
 {
@@ -81,7 +85,42 @@  namespace pmr
 
     constant_init<newdel_res_t> newdel_res{};
     constant_init<null_res_t> null_res{};
-    constant_init<atomic<memory_resource*>> default_res{&newdel_res.obj};
+#if ATOMIC_POINTER_LOCK_FREE == 2
+    using atomic_mem_res = atomic<memory_resource*>;
+# define _GLIBCXX_ATOMIC_MEM_RES_CAN_BE_CONSTANT_INITIALIZED
+#else
+    // Can't use pointer-width atomics, define a type using a mutex instead:
+    struct atomic_mem_res
+    {
+# ifdef __GTHREAD_MUTEX_INIT
+#  define _GLIBCXX_ATOMIC_MEM_RES_CAN_BE_CONSTANT_INITIALIZED
+      // std::mutex has constexpr constructor
+      constexpr
+# endif
+      atomic_mem_res(memory_resource* r) : val(r) { }
+
+      mutex mx;
+      memory_resource* val;
+
+      memory_resource* load()
+      {
+	lock_guard<mutex> lock(mx);
+	return val;
+      }
+
+      memory_resource* exchange(memory_resource* r)
+      {
+	lock_guard<mutex> lock(mx);
+	return std::exchange(val, r);
+      }
+    };
+#endif // ATOMIC_POINTER_LOCK_FREE == 2
+
+#ifdef _GLIBCXX_ATOMIC_MEM_RES_CAN_BE_CONSTANT_INITIALIZED
+    constant_init<atomic_mem_res> default_res{&newdel_res.obj};
+#else
+# include "default_resource.h"
+#endif
   } // namespace
 
   memory_resource*