version 27 fail to build with static assertion failed: 128-bit atomic operations are not lock-fre about fmtconv

[EleonoreMizo]

Thanks for the report. In src/conc/Interlocked.hpp, line 328, could you try to replace:

	static_assert (
		__atomic_always_lock_free (sizeof (dest), nullptr),
		"128-bit atomic operations are not lock-free"
	);
	old = comp;
	__atomic_compare_exchange_n (
		&dest, &old, excg,
		false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST
	);

with:

	old = __sync_val_compare_and_swap (&dest, comp, excg);

and tell me if it works better? Thanks

You could also try with clang: configure --enable-clang

[marillat]

This patch is correct ? If yes this doesn't build. See the build error after the patch ?

--- fmtconv-dmo-27.orig/src/conc/Interlocked.hpp
+++ fmtconv-dmo-27/src/conc/Interlocked.hpp
@@ -325,7 +325,7 @@ void        Interlocked::cas (Data128 &old, vol
 
 #elif defined (__GNUC__)
 
-       static_assert (
+       old = __sync_val_compare_and_swap (&dest, comp, excg);
                __atomic_always_lock_free (sizeof (dest), nullptr),
                "128-bit atomic operations are not lock-free"
        );

GCC output.

libtool: compile:  g++ -DPACKAGE_NAME=\"fmtconv\" -DPACKAGE_TARNAME=\"fmtconv\" -DPACKAGE_VERSION=\"r27\" "-DPACKAGE_STRING=\"fmtconv r27\"" "-DPACKAGE_BUGREPORT=\"http://forum.doom9.org/showthread.php?t=166504\"" "-DPACKAGE_URL=\"http://forum.doom9.org/showthread.php?t=166504\"" -DHAVE_STDIO_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_STRINGS_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_UNISTD_H=1 -DSTDC_HEADERS=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" -I. -Wdate-time -D_FORTIFY_SOURCE=2 -std=c++14 -O3 -g3 -DNDEBUG -mfpmath=sse -msse2 -mcx16 -Wall -Wextra -Wshadow -Wunused -Wnull-dereference -Wvla -Wstrict-aliasing -Wuninitialized -Wunused-parameter -Wreorder -Wsign-compare -Wunreachable-code -Wconversion -Wno-sign-conversion -Wredundant-decls -Wno-ignored-attributes -Wno-expansion-to-defined -I./../../src -g -O2 -ffile-prefix-map=/home/marillat/src/fmtconv-27=. -fstack-protector-strong -Wformat -Werror=format-security -c ../../src/fmtcl/Dither.cpp  -fPIC -DPIC -o ../../src/fmtcl/.libs/Dither.o
In file included from ./../../src/conc/Interlocked.h:132,
                 from ./../../src/conc/AtomicPtr.hpp:27,
                 from ./../../src/conc/AtomicPtr.h:123,
                 from ./../../src/conc/CellPool.h:30,
                 from ./../../src/conc/ObjPool.h:44,
                 from ./../../src/fmtcl/Dither.h:26,
                 from ../../src/fmtcl/Dither.cpp:29:
./../../src/conc/Interlocked.hpp: In static member function 'static void conc::Interlocked::cas(conc::Interlocked::Data128&, volatile Data128&, const Data128&, const Data128&)':
./../../src/conc/Interlocked.hpp:329:43: warning: left operand of comma operator has no effect [-Wunused-value]
  329 |                 __atomic_always_lock_free (sizeof (dest), nullptr),
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
./../../src/conc/Interlocked.hpp:330:62: error: expected ';' before ')' token
  330 |                 "128-bit atomic operations are not lock-free"
      |                                                              ^
      |                                                              ;
  331 |         );
      |         ~                                                     
./../../src/conc/Interlocked.hpp:331:10: warning: right operand of comma operator has no effect [-Wunused-value]
  331 |         );
      |          ^
make: *** [Makefile:1779: ../../src/fmtcl/Dither.lo] Error 1

[marillat]

Well me be the patch is supposed to be this one ?

--- a/src/conc/Interlocked.hpp
+++ b/src/conc/Interlocked.hpp
@@ -325,15 +325,7 @@ void	Interlocked::cas (Data128 &old, vol
 
 #elif defined (__GNUC__)
 
-	static_assert (
-		__atomic_always_lock_free (sizeof (dest), nullptr),
-		"128-bit atomic operations are not lock-free"
-	);
-	old = comp;
-	__atomic_compare_exchange_n (
-		&dest, &old, excg,
-		false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST
-	);
+	old = __sync_val_compare_and_swap (&dest, comp, excg);
 
 #else

[EleonoreMizo]

Yes, the second one.

[marillat]

This fix this issue for amd64, arm64, armhf and i 86 arches but fail with armel for gcc and clang.

libtool: compile:  clang++ -DPACKAGE_NAME=\"fmtconv\" -DPACKAGE_TARNAME=\"fmtconv\" -DPACKAGE_VERSION=\"r27\" "-DPACKAGE_STRING=\"fmtconv r27\"" "-DPACKAGE_BUGREPORT=\"http://forum.doom9.org/showthread.php?t=166504\"" "-DPACKAGE_URL=\"http://forum.doom9.org/showthread.php?t=166504\"" -DHAVE_STDIO_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_STRINGS_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_UNISTD_H=1 -DSTDC_HEADERS=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" -I. -Wdate-time -D_FORTIFY_SOURCE=2 -std=c++14 -O3 -g3 -DNDEBUG -ftree-vectorize -mfpu=neon -Wall -Wextra -Wshadow -Wunused -Wnull-dereference -Wvla -Wstrict-aliasing -Wuninitialized -Wunused-parameter -Wreorder -Wsign-compare -Wunreachable-code -Wconversion -Wno-sign-conversion -Wshadow-all -Wshorten-64-to-32 -Wint-conversion -Wconditional-uninitialized -Wconstant-conversion -Wunused-private-field -Wbool-conversion -Wextra-semi -Wnullable-to-nonnull-conversion -Wno-unused-private-field -Wno-unused-command-line-argument -I./../../src -g -O2 -ffile-prefix-map=/home/marillat/fmtconv-dmo-27=. -fstack-protector-strong -Wformat -Werror=format-security -c ../../src/fmtcl/Dither.cpp  -fPIC -DPIC -o ../../src/fmtcl/.libs/Dither.o
In file included from ../../src/fmtcl/Dither.cpp:29:
In file included from ./../../src/fmtcl/Dither.h:26:
In file included from ./../../src/conc/ObjPool.h:44:
In file included from ./../../src/conc/CellPool.h:33:
In file included from ./../../src/conc/LockFreeStack.h:45:
In file included from ./../../src/conc/AtomicPtrIntPair.h:40:
In file included from ./../../src/conc/Interlocked.h:132:
./../../src/conc/Interlocked.hpp:105:2: error: static_assert failed due to requirement '__atomic_always_lock_free(sizeof (dest), nullptr)' "32-bit atomic operations are not lock-free"
        static_assert (
        ^
./../../src/conc/Interlocked.hpp:223:2: error: static_assert failed due to requirement '__atomic_always_lock_free(sizeof (dest), nullptr)' "64-bit atomic operations are not lock-free"
        static_assert (
        ^
2 errors generated.
make: *** [Makefile:1779: ../../src/fmtcl/Dither.lo] Error 1

[EleonoreMizo]

Thank you so much for the testing. It seems armel architecture doesn’t have lock-free primitives, so I think I’ll just remove the static_assert() so the compiler will make use of the non-lock-free fallback. This will be sub-optimal but should work.

[marillat]

master commit 3a78392 fail to build for arm64

ERROR: compile ../../src/fmtcl/Dither.cpp on localhost failed
In file included from ./../../src/conc/Interlocked.h:132,
                 from ./../../src/conc/AtomicPtrIntPair.h:40,
                 from ./../../src/conc/LockFreeStack.h:45,
                 from ./../../src/conc/CellPool.h:33,
                 from ./../../src/conc/ObjPool.h:44,
                 from ./../../src/fmtcl/Dither.h:26,
                 from ../../src/fmtcl/Dither.cpp:29:
./../../src/conc/Interlocked.hpp: In static member function 'static void conc::Interlocked::cas(conc::Interlocked::Data128&, volatile Data128&, const Data128&, const Data128&)':
./../../src/conc/Interlocked.hpp:335:51: error: static assertion failed: 128-bit atomic operations are not lock-free
  335 |                         __atomic_always_lock_free (sizeof (dest), nullptr),
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
make: *** [Makefile:1805: ../../src/fmtcl/Dither.lo] Error 1

[EleonoreMizo]

The hell… Is 86d5b90 better?

[marillat]

Yes, build for all arches.