Lengthy discussion : https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59448
Basically we can generate incorrect code for an atomic consume operation
in some circumstances. The general feeling seems to be that we should
simply promote all consume operations to an acquire operation until
there is a better definition/understanding of the consume model and how
GCC can track it.
I proposed a simple patch in the PR, and I have not seen or heard of any
dissenting opinion. We should get this in before the end of stage 3 I
think.
The problem with the patch in the PR is the memory model is immediately
promoted from consume to acquire. This happens *before* any of the
memmodel checks are made. If a consume is illegally specified (such as
in a compare_exchange), it gets promoted to acquire and the compiler
doesn't report the error because it never sees the consume.
This new patch simply makes the adjustment after any errors are checked
on the originally specified model. It bootstraps on
x86_64-unknown-linux-gnu and passes all regression testing.
I also built an aarch64 compiler and it appears to issue the LDAR as
specified in the PR, but anyone with a vested interest really ought to
check it out with a real build to be sure.
OK for trunk?
Andrew
* builtins.c (memmodel_consume_fix) : New. Promote consume to acquire.
(expand_builtin_atomic_exchange, expand_builtin_atomic_compare_exchange,
expand_builtin_atomic_load, expand_builtin_atomic_fetch_op,
expand_builtin_atomic_clear, expand_builtin_atomic_test_and_set,
expand_builtin_atomic_thread_fence, expand_builtin_atomic_signal_fence):
Call memmodel_consume_fix.
Index: builtins.c
===================================================================
*** builtins.c (revision 219462)
--- builtins.c (working copy)
*************** get_memmodel (tree exp)
*** 5368,5373 ****
--- 5368,5382 ----
return (enum memmodel) val;
}
+ /* Workaround for Bugzilla 59448. GCC doesn't track consume properly, so
+ be conservative and promote consume to acquire. */
+ static void
+ memmodel_consume_fix (enum memmodel &val)
+ {
+ if (val == MEMMODEL_CONSUME)
+ val = MEMMODEL_ACQUIRE;
+ }
+
/* Expand the __atomic_exchange intrinsic:
TYPE __atomic_exchange (TYPE *object, TYPE desired, enum memmodel)
EXP is the CALL_EXPR.
*************** expand_builtin_atomic_exchange (machine_
*** 5389,5394 ****
--- 5398,5405 ----
if (!flag_inline_atomics)
return NULL_RTX;
+ memmodel_consume_fix (model);
+
/* Expand the operands. */
mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
val = expand_expr_force_mode (CALL_EXPR_ARG (exp, 1), mode);
*************** expand_builtin_atomic_compare_exchange (
*** 5434,5439 ****
--- 5445,5453 ----
if (!flag_inline_atomics)
return NULL_RTX;
+ memmodel_consume_fix (success);
+ memmodel_consume_fix (failure);
+
/* Expand the operands. */
mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
*************** expand_builtin_atomic_load (machine_mode
*** 5493,5498 ****
--- 5507,5514 ----
if (!flag_inline_atomics)
return NULL_RTX;
+ memmodel_consume_fix (model);
+
/* Expand the operand. */
mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
*************** expand_builtin_atomic_fetch_op (machine_
*** 5553,5558 ****
--- 5569,5576 ----
model = get_memmodel (CALL_EXPR_ARG (exp, 2));
+ memmodel_consume_fix (model);
+
/* Expand the operands. */
mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
val = expand_expr_force_mode (CALL_EXPR_ARG (exp, 1), mode);
*************** expand_builtin_atomic_clear (tree exp)
*** 5627,5632 ****
--- 5645,5652 ----
return const0_rtx;
}
+ memmodel_consume_fix (model);
+
if (HAVE_atomic_clear)
{
emit_insn (gen_atomic_clear (mem, model));
*************** expand_builtin_atomic_test_and_set (tree
*** 5658,5664 ****
mode = mode_for_size (BOOL_TYPE_SIZE, MODE_INT, 0);
mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
model = get_memmodel (CALL_EXPR_ARG (exp, 1));
!
return expand_atomic_test_and_set (target, mem, model);
}
--- 5678,5684 ----
mode = mode_for_size (BOOL_TYPE_SIZE, MODE_INT, 0);
mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
model = get_memmodel (CALL_EXPR_ARG (exp, 1));
! memmodel_consume_fix (model);
return expand_atomic_test_and_set (target, mem, model);
}
*************** static void
*** 5797,5802 ****
--- 5817,5823 ----
expand_builtin_atomic_thread_fence (tree exp)
{
enum memmodel model = get_memmodel (CALL_EXPR_ARG (exp, 0));
+ memmodel_consume_fix (model);
expand_mem_thread_fence (model);
}
*************** static void
*** 5808,5813 ****
--- 5829,5835 ----
expand_builtin_atomic_signal_fence (tree exp)
{
enum memmodel model = get_memmodel (CALL_EXPR_ARG (exp, 0));
+ memmodel_consume_fix (model);
expand_mem_signal_fence (model);
}
