On 01/13/2015 05:37 PM, Joseph Myers wrote:
On Tue, 13 Jan 2015, Andrew MacLeod wrote:
It seems that it should be safe to move back to the original patch, and remove
that error test for using consume on an exchange...
I don't think there should be any such errors, for any of the atomic
built-in functions, only warnings (with the model converted to
MEMMODEL_SEQ_CST if not valid, just like a non-constant model). This is
just like any other invalid function argument of a suitable type:
undefined behavior only if the call is executed.
http://www.open-std.org/jtc1/sc22/wg14/12553
Can't see what is in the link, but we've discussed this before.
- There is a warning for invalid memory models already, so I just
continued using that.
- I remove the check for CONSUME in exchange since the current standard
makes no mention of that being illegal.
- I also reversed the current check in compare_exchange to check for
failure > success first, allowing us to still catch both errors if present.
I think this brings us to where we ought to be... at least almost :-)
The latest version I have is n3337, which still specifies that
atomic_clear can't be memory_order_acquire or memory_order_acq_rel. Has
that been updated to specify that memory_order_consume is not allowed
either? I think there was a request in at some point... I can add
that if so.
Bootstraps on x86_64-unknown-linux-gnu, and no regressions in the testsuite.
OK for trunk?
Andrew
2015-01-14 Andrew MacLeod <amacl...@redhat.com>
* builtins.c (expand_builtin_atomic_exchange): Remove error when
memory model is CONSUME.
(expand_builtin_atomic_compare_exchange, expand_builtin_atomic_load,
expand_builtin_atomic_store, expand_builtin_atomic_clear): Change
invalid memory model errors to warnings.
* testsuite/gcc.dg/atomic-invalid.c: Check for invalid memory model
warnings instead of errors.
Index: builtins.c
===================================================================
*** builtins.c (revision 219601)
--- builtins.c (working copy)
*************** expand_builtin_atomic_exchange (machine_
*** 5385,5395 ****
enum memmodel model;
model = get_memmodel (CALL_EXPR_ARG (exp, 2));
- if ((model & MEMMODEL_MASK) == MEMMODEL_CONSUME)
- {
- error ("invalid memory model for %<__atomic_exchange%>");
- return NULL_RTX;
- }
if (!flag_inline_atomics)
return NULL_RTX;
--- 5385,5390 ----
*************** expand_builtin_atomic_compare_exchange (
*** 5422,5441 ****
success = get_memmodel (CALL_EXPR_ARG (exp, 4));
failure = get_memmodel (CALL_EXPR_ARG (exp, 5));
if ((failure & MEMMODEL_MASK) == MEMMODEL_RELEASE
|| (failure & MEMMODEL_MASK) == MEMMODEL_ACQ_REL)
{
! error ("invalid failure memory model for %<__atomic_compare_exchange%>");
! return NULL_RTX;
}
! if (failure > success)
! {
! error ("failure memory model cannot be stronger than success "
! "memory model for %<__atomic_compare_exchange%>");
! return NULL_RTX;
! }
!
if (!flag_inline_atomics)
return NULL_RTX;
--- 5417,5441 ----
success = get_memmodel (CALL_EXPR_ARG (exp, 4));
failure = get_memmodel (CALL_EXPR_ARG (exp, 5));
+ if (failure > success)
+ {
+ warning (OPT_Winvalid_memory_model,
+ "failure memory model cannot be stronger than success memory "
+ "model for %<__atomic_compare_exchange%>");
+ success = MEMMODEL_SEQ_CST;
+ }
+
if ((failure & MEMMODEL_MASK) == MEMMODEL_RELEASE
|| (failure & MEMMODEL_MASK) == MEMMODEL_ACQ_REL)
{
! warning (OPT_Winvalid_memory_model,
! "invalid failure memory model for "
! "%<__atomic_compare_exchange%>");
! failure = MEMMODEL_SEQ_CST;
! success = MEMMODEL_SEQ_CST;
}
!
if (!flag_inline_atomics)
return NULL_RTX;
*************** expand_builtin_atomic_load (machine_mode
*** 5491,5498 ****
if ((model & MEMMODEL_MASK) == MEMMODEL_RELEASE
|| (model & MEMMODEL_MASK) == MEMMODEL_ACQ_REL)
{
! error ("invalid memory model for %<__atomic_load%>");
! return NULL_RTX;
}
if (!flag_inline_atomics)
--- 5491,5499 ----
if ((model & MEMMODEL_MASK) == MEMMODEL_RELEASE
|| (model & MEMMODEL_MASK) == MEMMODEL_ACQ_REL)
{
! warning (OPT_Winvalid_memory_model,
! "invalid memory model for %<__atomic_load%>");
! model = MEMMODEL_SEQ_CST;
}
if (!flag_inline_atomics)
*************** expand_builtin_atomic_store (machine_mod
*** 5521,5528 ****
&& (model & MEMMODEL_MASK) != MEMMODEL_SEQ_CST
&& (model & MEMMODEL_MASK) != MEMMODEL_RELEASE)
{
! error ("invalid memory model for %<__atomic_store%>");
! return NULL_RTX;
}
if (!flag_inline_atomics)
--- 5522,5530 ----
&& (model & MEMMODEL_MASK) != MEMMODEL_SEQ_CST
&& (model & MEMMODEL_MASK) != MEMMODEL_RELEASE)
{
! warning (OPT_Winvalid_memory_model,
! "invalid memory model for %<__atomic_store%>");
! model = MEMMODEL_SEQ_CST;
}
if (!flag_inline_atomics)
*************** expand_builtin_atomic_clear (tree exp)
*** 5628,5635 ****
if ((model & MEMMODEL_MASK) == MEMMODEL_ACQUIRE
|| (model & MEMMODEL_MASK) == MEMMODEL_ACQ_REL)
{
! error ("invalid memory model for %<__atomic_store%>");
! return const0_rtx;
}
if (HAVE_atomic_clear)
--- 5630,5638 ----
if ((model & MEMMODEL_MASK) == MEMMODEL_ACQUIRE
|| (model & MEMMODEL_MASK) == MEMMODEL_ACQ_REL)
{
! warning (OPT_Winvalid_memory_model,
! "invalid memory model for %<__atomic_store%>");
! model = MEMMODEL_SEQ_CST;
}
if (HAVE_atomic_clear)
Index: testsuite/gcc.dg/atomic-invalid.c
===================================================================
*** testsuite/gcc.dg/atomic-invalid.c (revision 219601)
--- testsuite/gcc.dg/atomic-invalid.c (working copy)
*************** bool x;
*** 13,35 ****
int
main ()
{
! __atomic_compare_exchange_n (&i, &e, 1, 0, __ATOMIC_RELAXED, __ATOMIC_SEQ_CST); /* { dg-error "failure memory model cannot be stronger" } */
! __atomic_compare_exchange_n (&i, &e, 1, 0, __ATOMIC_SEQ_CST, __ATOMIC_RELEASE); /* { dg-error "invalid failure memory" } */
! __atomic_compare_exchange_n (&i, &e, 1, 1, __ATOMIC_SEQ_CST, __ATOMIC_ACQ_REL); /* { dg-error "invalid failure memory" } */
!
! __atomic_load_n (&i, __ATOMIC_RELEASE); /* { dg-error "invalid memory model" } */
! __atomic_load_n (&i, __ATOMIC_ACQ_REL); /* { dg-error "invalid memory model" } */
!
! __atomic_store_n (&i, 1, __ATOMIC_ACQUIRE); /* { dg-error "invalid memory model" } */
! __atomic_store_n (&i, 1, __ATOMIC_CONSUME); /* { dg-error "invalid memory model" } */
! __atomic_store_n (&i, 1, __ATOMIC_ACQ_REL); /* { dg-error "invalid memory model" } */
i = __atomic_always_lock_free (s, NULL); /* { dg-error "non-constant argument" } */
__atomic_load_n (&i, 44); /* { dg-warning "invalid memory model" } */
! __atomic_clear (&x, __ATOMIC_ACQUIRE); /* { dg-error "invalid memory model" } */
! __atomic_clear (&x, __ATOMIC_ACQ_REL); /* { dg-error "invalid memory model" } */
}
--- 13,35 ----
int
main ()
{
! __atomic_compare_exchange_n (&i, &e, 1, 0, __ATOMIC_RELAXED, __ATOMIC_SEQ_CST); /* { dg-warning "failure memory model cannot be stronger" } */
! __atomic_compare_exchange_n (&i, &e, 1, 0, __ATOMIC_SEQ_CST, __ATOMIC_RELEASE); /* { dg-warning "invalid failure memory" } */
! __atomic_compare_exchange_n (&i, &e, 1, 1, __ATOMIC_SEQ_CST, __ATOMIC_ACQ_REL); /* { dg-warning "invalid failure memory" } */
!
! __atomic_load_n (&i, __ATOMIC_RELEASE); /* { dg-warning "invalid memory model" } */
! __atomic_load_n (&i, __ATOMIC_ACQ_REL); /* { dg-warning "invalid memory model" } */
!
! __atomic_store_n (&i, 1, __ATOMIC_ACQUIRE); /* { dg-warning "invalid memory model" } */
! __atomic_store_n (&i, 1, __ATOMIC_CONSUME); /* { dg-warning "invalid memory model" } */
! __atomic_store_n (&i, 1, __ATOMIC_ACQ_REL); /* { dg-warning "invalid memory model" } */
i = __atomic_always_lock_free (s, NULL); /* { dg-error "non-constant argument" } */
__atomic_load_n (&i, 44); /* { dg-warning "invalid memory model" } */
! __atomic_clear (&x, __ATOMIC_ACQUIRE); /* { dg-warning "invalid memory model" } */
! __atomic_clear (&x, __ATOMIC_ACQ_REL); /* { dg-warning "invalid memory model" } */
}
