Patchwork [2/2] Support __ATOMIC_HLE_RELEASE for __atomic_clear/store_n

login
register
mail settings
Submitter Uros Bizjak
Date Jan. 13, 2013, 10:23 p.m.
Message ID <CAFULd4ae7eqmyha0UrtL-ECNh_k9KXv0eYp9B1==Y+ELYNcMRA@mail.gmail.com>
Download mbox | patch
Permalink /patch/211658/
State New
Headers show

Comments

Uros Bizjak - Jan. 13, 2013, 10:23 p.m.
On Sun, Jan 13, 2013 at 11:12 PM, Andi Kleen <andi@firstfloor.org> wrote:
>> >> +(define_insn "atomic_store<mode>_1"
>> >> +  [(set (match_operand:ATOMIC 0 "memory_operand" "=m")
>> >> +     (unspec:ATOMIC [(match_operand:ATOMIC 1 "<nonmemory_operand>" "<r><i>")
>> >> +                     (match_operand:SI 2 "const_int_operand")]
>> >> +                    UNSPEC_MOVA))]
>> >> +  ""
>> >> +  "%K2mov{<imodesuffix>}\t{%1, %0|%0, %1}")
>> >
>> > Is that the updated pattern you wanted? It looks similar to mine.
>>
>> Yes the attached patch actually implements all proposed fixes.
>
> Ok great. Can you just commit it then? It looks good to me.

No problem, but what about this part:


Middle-end support should be implemented before target support is
committed. So, please figure out how to emit correct error on
unsupported models and get middle-end patch reviewed first. We do get
"Error: instruction `mov' after `xacquire' not allowed" assembler
error with "xacquire movb $0,mem" asm, though.

Uros.
Andi Kleen - Jan. 13, 2013, 10:29 p.m.
On Sun, Jan 13, 2013 at 11:23:24PM +0100, Uros Bizjak wrote:
> On Sun, Jan 13, 2013 at 11:12 PM, Andi Kleen <andi@firstfloor.org> wrote:
> >> >> +(define_insn "atomic_store<mode>_1"
> >> >> +  [(set (match_operand:ATOMIC 0 "memory_operand" "=m")
> >> >> +     (unspec:ATOMIC [(match_operand:ATOMIC 1 "<nonmemory_operand>" "<r><i>")
> >> >> +                     (match_operand:SI 2 "const_int_operand")]
> >> >> +                    UNSPEC_MOVA))]
> >> >> +  ""
> >> >> +  "%K2mov{<imodesuffix>}\t{%1, %0|%0, %1}")
> >> >
> >> > Is that the updated pattern you wanted? It looks similar to mine.
> >>
> >> Yes the attached patch actually implements all proposed fixes.
> >
> > Ok great. Can you just commit it then? It looks good to me.
> 
> No problem, but what about this part:

Right now it just means its silently ignored, no wrong code generated.
If people are ok with a new target hook I can add one.
There are some more bugs in this area, like PR55947

Giving a warning is imho less important than supporting this at all.
So I would prefer to not delay this patch.

> 
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 2b615a1..c283869 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -5556,6 +5556,8 @@ expand_builtin_atomic_clear (tree exp)
>        return const0_rtx;
>      }
> 
> +  /* need target hook there to check for not hle acquire */
> +
>    if (HAVE_atomic_clear)
>      {
>        emit_insn (gen_atomic_clear (mem, model));
> 
> Middle-end support should be implemented before target support is
> committed. So, please figure out how to emit correct error on
> unsupported models and get middle-end patch reviewed first. We do get
> "Error: instruction `mov' after `xacquire' not allowed" assembler
> error with "xacquire movb $0,mem" asm, though.

The sync.md code is only called for the acquire bit.

The only case where it may happen I guess if someone sets both.


-Andi

Patch

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 2b615a1..c283869 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -5556,6 +5556,8 @@  expand_builtin_atomic_clear (tree exp)
       return const0_rtx;
     }

+  /* need target hook there to check for not hle acquire */
+
   if (HAVE_atomic_clear)
     {
       emit_insn (gen_atomic_clear (mem, model));