Hi!
If the val operand due to TER is a MEM (or contains a MEM) which can alias
with mem (the memory atomically updated), we have various wrong-code
problems in the fallback expansions.
The first one can be seen on the testcase and is solved by the second/third
hunks, if the backend has an instruction to atomically update in the mode
and return the previous rather than the updated value, we perform non-atomic
update of that value with val again after the atomic insn. If val is or
overlaps with mem, we then incorrectly use an updated value rather than
the original value (of course, the code is weird because it non-atomically
reads first what is then atomically updated, so if some other thread
modifies it, what will be the result is pretty random). This patch changes
- movl v(%rip), %eax
+ movl v(%rip), %edx
+ movl %edx, %eax
lock xaddl %eax, v(%rip)
- addl v(%rip), %eax
+ addl %edx, %eax
cmpl $18, %eax
jne .L3
xorl %eax, %eax
ret
on x86_64 by forcing the val into pseudo in that case. Another problem
(the last 2 hunks in optabs.cc) is if there is no named pattern for it
and we fallback to compare and exchange loop. E.g. if similar testcase
uses unsigned long long instead of unsigned on x86_64 -m32 -march=x86-64.
Not included in testsuite because it actually "works" on uncontended
variable, the bug is that val is read from memory twice, once before
the loop and once inside of the loop before the cmpxchg8b instruction.
So it is actually correct unless cmpxchg8b fails the first time.
And the first hunk is to do the same thing on various other places that
use cmpxchg fallback loop.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2026-09-12 Jakub Jelinek <[email protected]>
PR middle-end/127336
* optabs.cc (maybe_emit_compare_and_swap_exchange_loop,
expand_atomic_fetch_op_no_fallback, expand_atomic_fetch_op): Force
val into a pseduo if it updates to mem could change its value
and when using val after the atomic op or inside of a loop.
* gcc.dg/pr127336.c: New test.
--- a/gcc/optabs.cc 2026-09-08 12:52:03.000000000 +0200
+++ b/gcc/optabs.cc 2026-09-11 18:45:50.758771839 +0200
@@ -7354,10 +7354,17 @@ maybe_emit_compare_and_swap_exchange_loo
if (can_compare_and_swap_p (mode, true))
{
+ rtx_insn *start = get_last_insn ();
+ /* Force val into a register if it could change value when the
+ atomic insn updates mem. */
+ if (reg_overlap_mentioned_p (mem, val))
+ val = force_reg (mode, val);
if (!target || !register_operand (target, mode))
target = gen_reg_rtx (mode);
if (expand_compare_and_swap_loop (mem, target, val, NULL_RTX))
return target;
+ else
+ delete_insns_since (start);
}
return NULL_RTX;
@@ -8153,6 +8160,12 @@ expand_atomic_fetch_op_no_fallback (rtx
try that operation. */
if (after || unused_result || optab.reverse_code != UNKNOWN)
{
+ rtx_insn *start = get_last_insn ();
+ /* Force val into a register if it could change value when the
+ atomic insn updates mem. */
+ if (!unused_result && reg_overlap_mentioned_p (mem, val))
+ val = force_reg (mode, val);
+
/* Try the __atomic version, then the older __sync version. */
result = maybe_emit_op (&optab, target, mem, val, true, model, !after);
if (!result)
@@ -8179,6 +8192,8 @@ expand_atomic_fetch_op_no_fallback (rtx
true, OPTAB_LIB_WIDEN);
return result;
}
+ else
+ delete_insns_since (start);
}
/* No direct opcode can be generated. */
@@ -8278,6 +8293,11 @@ expand_atomic_fetch_op (rtx target, rtx
{
rtx_insn *insn;
rtx t0 = gen_reg_rtx (mode), t1;
+ rtx_insn *start = get_last_insn ();
+ /* Force val into a register if it could change value when the
+ atomic insn updates mem. */
+ if (reg_overlap_mentioned_p (mem, val))
+ val = force_reg (mode, val);
start_sequence ();
@@ -8311,6 +8331,8 @@ expand_atomic_fetch_op (rtx target, rtx
if (t1 != NULL && expand_compare_and_swap_loop (mem, t0, t1, insn))
return target;
+ else
+ delete_insns_since (start);
}
return NULL_RTX;
--- a/gcc/testsuite/gcc.dg/pr127336.c 2026-09-11 18:54:50.342667339 +0200
+++ b/gcc/testsuite/gcc.dg/pr127336.c 2026-09-11 18:54:39.062815861 +0200
@@ -0,0 +1,12 @@
+/* PR middle-end/127336 */
+/* { dg-do run { target sync_int_long } } */
+/* { dg-options "-O2" } */
+
+unsigned v = 9;
+
+int
+main ()
+{
+ if (__atomic_add_fetch (&v, v, __ATOMIC_RELAXED) != 18)
+ __builtin_abort ();
+}
Jakub
