From: Jakub Jelinek <jakub@redhat.com>
Date: Wed, 27 Nov 2024 13:33:16 +0000 (+0100)
Subject: builtins: Emit __sync_lock_release_{8,16} call as last resort instead of doing nothin... 
X-Git-Tag: basepoints/gcc-16~3829
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=96ccb208e43a7cb7a1b15e28c94d57661bf9caae;p=thirdparty%2Fgcc.git

builtins: Emit __sync_lock_release_{8,16} call as last resort instead of doing nothing [PR117642]

As the following testcases show, in case of multi-word
__sync_lock_test_and_set where we don't actually support atomics for that
size (__int128 for x86_64 lp64 with -mno-cx16, long long for ia32 with
-march=i{3,4}86), as the last fallback if we don't know anything else
we just emit calls to __sync_lock_test_and_set_{8,16}.  Those aren't defined
in libatomic, but perhaps users could define them themselves.
While __sync_lock_release if it gives up and has no way to emit the atomic
store just does nothing at all, so no clear sign to the users something
went wrong and that the code will not do what they expected.
This regressed when __atomic_* support has been introduced, previously we
would just emit those calls even in this case.

The patch just emits the call as the last fallback.

2024-11-27  Jakub Jelinek  <jakub@redhat.com>

	PR target/117642
	* builtins.cc (expand_builtin_sync_lock_release): Change return type
	from void to rtx, return result of expand_atomic_store.
	(expand_builtin) <case BUILT_IN_SYNC_LOCK_RELEASE_16>: If
	expand_builtin_sync_lock_release returns NULL, do a break rather
	than return const0_rtx.

	* gcc.target/i386/pr117642-1.c: New test.
	* gcc.target/i386/pr117642-2.c: New test.
---

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 6271cdc1debb..d338abe95baa 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -6587,7 +6587,7 @@ expand_builtin_sync_lock_test_and_set (machine_mode mode, tree exp,
 
 /* Expand the __sync_lock_release intrinsic.  EXP is the CALL_EXPR.  */
 
-static void
+static rtx
 expand_builtin_sync_lock_release (machine_mode mode, tree exp)
 {
   rtx mem;
@@ -6595,7 +6595,7 @@ expand_builtin_sync_lock_release (machine_mode mode, tree exp)
   /* Expand the operands.  */
   mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
 
-  expand_atomic_store (mem, const0_rtx, MEMMODEL_SYNC_RELEASE, true);
+  return expand_atomic_store (mem, const0_rtx, MEMMODEL_SYNC_RELEASE, true);
 }
 
 /* Given an integer representing an ``enum memmodel'', verify its
@@ -8605,8 +8605,9 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
     case BUILT_IN_SYNC_LOCK_RELEASE_8:
     case BUILT_IN_SYNC_LOCK_RELEASE_16:
       mode = get_builtin_sync_mode (fcode - BUILT_IN_SYNC_LOCK_RELEASE_1);
-      expand_builtin_sync_lock_release (mode, exp);
-      return const0_rtx;
+      if (expand_builtin_sync_lock_release (mode, exp))
+	return const0_rtx;
+      break;
 
     case BUILT_IN_SYNC_SYNCHRONIZE:
       expand_builtin_sync_synchronize ();
diff --git a/gcc/testsuite/gcc.target/i386/pr117642-1.c b/gcc/testsuite/gcc.target/i386/pr117642-1.c
new file mode 100644
index 000000000000..c0b42faa2053
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr117642-1.c
@@ -0,0 +1,19 @@
+/* PR target/117642 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-mno-cx16" } */
+/* { dg-final { scan-assembler "__sync_lock_test_and_set_16" } } */
+/* { dg-final { scan-assembler "__sync_lock_release_16" } } */
+
+__int128 t = 1;
+
+void
+foo (void)
+{
+  __sync_lock_test_and_set (&t, 1);
+}
+
+void
+bar (void)
+{
+  __sync_lock_release (&t);
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr117642-2.c b/gcc/testsuite/gcc.target/i386/pr117642-2.c
new file mode 100644
index 000000000000..8a72a55384d6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr117642-2.c
@@ -0,0 +1,19 @@
+/* PR target/117642 */
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-march=i486" } */
+/* { dg-final { scan-assembler "__sync_lock_test_and_set_8" } } */
+/* { dg-final { scan-assembler "__sync_lock_release_8" } } */
+
+long long t = 1;
+
+void
+foo (void)
+{
+  __sync_lock_test_and_set (&t, 1);
+}
+
+void
+bar (void)
+{
+  __sync_lock_release (&t);
+}