Re: [PATCH 2/2] Support __ATOMIC_HLE_RELEASE for __atomic_clear/store_n

[Uros Bizjak <ubizjak at gmail dot com>]

On Sun, Jan 13, 2013 at 11:29 PM, Andi Kleen <andi@firstfloor.org> wrote:
> 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.

This cannot happen, we reject code that sets both __HLE* flags.

2012-01-14  Uros Bizjak  <ubizjak@gmail.com>
	    Andi Kleen  <ak@linux.intel.com>

	PR target/55948
	* config/i386/sync.md (atomic_store<mode>_1): New pattern.
	(atomic_store<mode>): Call atomic_store<mode>_1 for IX86_HLE_RELEASE
	memmodel flag.

testsuite/ChangeLog

2012-01-14  Andi Kleen  <ak@linux.intel.com>

	PR target/55948
	* gcc.target/i386/hle-clear-rel.c: New file
	* gcc.target/i386/hle-store-rel.c: New file.

I have committed attached patch to mainline SVN, after re-tested it on
x86_64-pc-linux-gnu.

Uros.

Attachment: p.diff.txt
Description: Text document