From: Uros Bizjak <ubizjak@gmail.com>
To: Andi Kleen <andi@firstfloor.org>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 2/2] Support __ATOMIC_HLE_RELEASE for __atomic_clear/store_n
Date: Mon, 14 Jan 2013 16:48:00 -0000 [thread overview]
Message-ID: <CAFULd4bWUhZ2ZLJA7Cs+oxxdPvfHbzcmMDgB7b11ZhZbD6JF1g@mail.gmail.com> (raw)
In-Reply-To: <20130113222946.GX30577@one.firstfloor.org>
[-- Attachment #1: Type: text/plain, Size: 2618 bytes --]
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 #2: p.diff.txt --]
[-- Type: text/plain, Size: 2153 bytes --]
Index: config/i386/sync.md
===================================================================
--- config/i386/sync.md (revision 195152)
+++ config/i386/sync.md (working copy)
@@ -224,8 +224,12 @@
DONE;
}
- /* Otherwise use a normal store. */
- emit_move_insn (operands[0], operands[1]);
+ /* Otherwise use a store. */
+ if (INTVAL (operands[2]) & IX86_HLE_RELEASE)
+ emit_insn (gen_atomic_store<mode>_1 (operands[0], operands[1],
+ operands[2]));
+ else
+ emit_move_insn (operands[0], operands[1]);
}
/* ... followed by an MFENCE, if required. */
if (model == MEMMODEL_SEQ_CST)
@@ -233,6 +237,14 @@
DONE;
})
+(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}")
+
(define_insn_and_split "atomic_storedi_fpu"
[(set (match_operand:DI 0 "memory_operand" "=m,m,m")
(unspec:DI [(match_operand:DI 1 "register_operand" "x,m,?r")]
Index: testsuite/gcc.target/i386/hle-clear-rel.c
===================================================================
--- testsuite/gcc.target/i386/hle-clear-rel.c (revision 0)
+++ testsuite/gcc.target/i386/hle-clear-rel.c (working copy)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-mhle" } */
+/* { dg-final { scan-assembler "\[ \n\t\]+\(xrelease\|\.byte\[ \t\]+0xf3\)\[ \t\n\]+mov" } } */
+
+void
+hle_clear (char *p, int v)
+{
+ __atomic_clear (p, __ATOMIC_RELEASE | __ATOMIC_HLE_RELEASE);
+}
Index: testsuite/gcc.target/i386/hle-store-rel.c
===================================================================
--- testsuite/gcc.target/i386/hle-store-rel.c (revision 0)
+++ testsuite/gcc.target/i386/hle-store-rel.c (working copy)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-mhle" } */
+/* { dg-final { scan-assembler "\[ \n\t\]+\(xrelease\|\.byte\[ \t\]+0xf3\)\[ \t\n\]+mov" } } */
+
+void
+hle_store (int *p, int v)
+{
+ __atomic_store_n (p, v, __ATOMIC_RELEASE | __ATOMIC_HLE_RELEASE);
+}
next prev parent reply other threads:[~2013-01-14 16:48 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-13 19:59 Uros Bizjak
2013-01-13 20:36 ` Andi Kleen
2013-01-13 20:59 ` Uros Bizjak
2013-01-13 22:13 ` Andi Kleen
2013-01-13 22:23 ` Uros Bizjak
2013-01-13 22:29 ` Andi Kleen
2013-01-14 16:48 ` Uros Bizjak [this message]
2013-01-14 18:06 ` Andi Kleen
2013-01-14 18:41 ` Uros Bizjak
2013-01-14 19:02 ` Andi Kleen
2013-01-14 19:21 ` Uros Bizjak
2013-01-14 19:25 ` Uros Bizjak
-- strict thread matches above, loose matches on Subject: below --
2013-01-12 15:29 [PATCH 1/2] Document HLE / RTM intrinsics Andi Kleen
2013-01-12 15:29 ` [PATCH 2/2] Support __ATOMIC_HLE_RELEASE for __atomic_clear/store_n Andi Kleen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFULd4bWUhZ2ZLJA7Cs+oxxdPvfHbzcmMDgB7b11ZhZbD6JF1g@mail.gmail.com \
--to=ubizjak@gmail.com \
--cc=andi@firstfloor.org \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).