On Mon, Jan 14, 2013 at 4:38 PM, Andi Kleen <a...@firstfloor.org> wrote:
> For HLE stores are only valid with __ATOMIC_HLE_RELEASE. The middle end
> didn't know this. This adds a new parameter to the get_memmodel target
> hook to distingush stores and give an warning for acquire stores.
>
> I also did a similar check for the barriers where HLE is not useful
> at all.
>
> Fixes one todo in the earlier hle release patch.
>
> gcc/:
> 2013-01-13 Andi Kleen <a...@linux.intel.com>
>
> PR target/55948
> * builtins.c (get_memmodel): Pass store, barrier parameters to target
> hook.
> (expand_builtin_atomic_exchange,
> expand_builtin_atomic_compare_exchange,
> expand_builtin_atomic_load,
> expand_builtin_atomic_store,
> expand_builtin_atomic_fetch_op,
> expand_builtin_atomic_test_and_set,
> expand_builtin_atomic_test_lock,
> expand_builtin_atomic_clear,
> expand_builtin_atomic_thread_fence): Pass store, barrier
> parameters to get_memmodel.
> * config/i386/i386.c (ix86_memmodel_check): Add store, barrier
> warning.
> * doc/tm.texi (TARGET_GET_MEMMODEL): Add store, barrier parameters.
> * doc/tm.exit.in (TARGET_GET_MEMMODEL): Dito.
> * target.def (TARGET_GET_MEMMODEL): Dito.
> ---
> gcc/builtins.c | 30 +++++++++++++++---------------
> gcc/config/i386/i386.c | 21 +++++++++++++++++++--
> gcc/doc/tm.texi | 6 ++++--
> gcc/doc/tm.texi.in | 4 +++-
> gcc/target.def | 3 ++-
> 5 files changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 4f778c1..50b6297 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -42076,14 +42076,31 @@ ix86_destroy_cost_data (void *data)
> free (data);
> }
>
> -/* Validate target specific memory model bits in VAL. */
> +/* Validate target specific memory model bits in VAL. When
> + STORE is true this for a store. When BARRIER is true
> + this is for a barrier. */
>
> static unsigned HOST_WIDE_INT
> -ix86_memmodel_check (unsigned HOST_WIDE_INT val)
> +ix86_memmodel_check (unsigned HOST_WIDE_INT val, bool store,
> + bool barrier)
> {
> unsigned HOST_WIDE_INT model = val & MEMMODEL_MASK;
> bool strong;
>
> + if (barrier && (val & (IX86_HLE_ACQUIRE|IX86_HLE_RELEASE)))
> + {
> + warning (OPT_Winvalid_memory_model,
> + "__ATOMIC_HLE_ACQUIRE or __ATOMIC_HLE_RELEASE not valid for
> barriers.");
No dot at the end of warning sentence.
> + return MEMMODEL_SEQ_CST;
> + }
> +
> + if (store && (val & IX86_HLE_ACQUIRE))
> + {
> + warning (OPT_Winvalid_memory_model,
> + "__ATOMIC_HLE_ACQUIRE not valid for stores.");
Also here.
The target part is OK, the rest needs a review from a middle-end reviewer.
Thanks,
Uros.
