From: Alexander Monakov <amonakov@ispras.ru>
To: Richard Sandiford <richard.sandiford@linaro.org>
Cc: gcc-patches@gcc.gnu.org, law@redhat.com
Subject: Re: [PATCH 1/3] optabs: ensure mem_thread_fence is a compiler barrier
Date: Mon, 07 Aug 2017 08:42:00 -0000 [thread overview]
Message-ID: <alpine.LNX.2.20.13.1708071135520.29517@monopod.intra.ispras.ru> (raw)
In-Reply-To: <877eyiidxr.fsf@linaro.org>
On Sat, 5 Aug 2017, Richard Sandiford wrote:
> It would be simpler to test whether targetm.gen_mem_thread_fence
> returns NULL.
>
> This feels a bit hacky though. Checking whether a generator produced no
> instructions is usually the test for whether the generator FAILed, which
> should normally be treated as though the pattern wasn't defined to start
> with. This is useful if the FAIL condition is too complex to put in the
> define_expand/insn C condition.
>
> In those contexts, if a generator wants to succeed and generate no
> instructions, it should emit a note instead. (Yes, it would be good
> to have a nicer interface...)
>
> So IMO it's confusing to overload the empty sequence to mean something
> else in this context, and it wouldn't work if a target does emit the kind
> of "I didn't FAIL" note just mentioned.
Thanks for the elucidation, I appreciate it.
> FWIW, I agree with Jeff in the previous thread that (compared to this)
> it would be more robust to make optabs.c emit the compile barrier
> unconditionally and just leave the target to emit a machine barrier.
OK, I've changed the patch accordingly. I've also factored out the
is_mm_relaxed check in this iteration to simplify resulting code.
PR target/80640
* doc/md.texi (mem_thread_fence): Remove mention of mode. Rewrite.
* optabs.c (expand_mem_thread_fence): Emit a compiler barrier when
using targetm.gen_mem_thread_fence.
testsuite/
* gcc.dg/atomic/pr80640.c: New testcase.
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index ea959208c98..64a137ecad0 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -7044,14 +7044,20 @@ If these patterns are not defined, attempts will be made to use
counterparts. If none of these are available a compare-and-swap
loop will be used.
-@cindex @code{mem_thread_fence@var{mode}} instruction pattern
-@item @samp{mem_thread_fence@var{mode}}
+@cindex @code{mem_thread_fence} instruction pattern
+@item @samp{mem_thread_fence}
This pattern emits code required to implement a thread fence with
memory model semantics. Operand 0 is the memory model to be used.
-If this pattern is not specified, all memory models except
-@code{__ATOMIC_RELAXED} will result in issuing a @code{sync_synchronize}
-barrier pattern.
+For the @code{__ATOMIC_RELAXED} model no instructions need to be issued
+and this expansion is not invoked.
+
+The compiler always emits a compiler memory barrier regardless of what
+expanding this pattern produced.
+
+If this pattern is not defined, the compiler falls back to expanding the
+@code{memory_barrier} pattern, then to emitting @code{__sync_synchronize}
+library call, and finally to just placing a compiler memory barrier.
@cindex @code{mem_signal_fence@var{mode}} instruction pattern
@item @samp{mem_signal_fence@var{mode}}
diff --git a/gcc/optabs.c b/gcc/optabs.c
index a9900657a58..71b74dd5feb 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -6297,17 +6297,19 @@ expand_asm_memory_barrier (void)
void
expand_mem_thread_fence (enum memmodel model)
{
+ if (is_mm_relaxed (model))
+ return;
if (targetm.have_mem_thread_fence ())
- emit_insn (targetm.gen_mem_thread_fence (GEN_INT (model)));
- else if (!is_mm_relaxed (model))
{
- if (targetm.have_memory_barrier ())
- emit_insn (targetm.gen_memory_barrier ());
- else if (synchronize_libfunc != NULL_RTX)
- emit_library_call (synchronize_libfunc, LCT_NORMAL, VOIDmode, 0);
- else
- expand_asm_memory_barrier ();
+ emit_insn (targetm.gen_mem_thread_fence (GEN_INT (model)));
+ expand_asm_memory_barrier ();
}
+ else if (targetm.have_memory_barrier ())
+ emit_insn (targetm.gen_memory_barrier ());
+ else if (synchronize_libfunc != NULL_RTX)
+ emit_library_call (synchronize_libfunc, LCT_NORMAL, VOIDmode, 0);
+ else
+ expand_asm_memory_barrier ();
}
/* This routine will either emit the mem_signal_fence pattern or issue a
diff --git a/gcc/testsuite/gcc.dg/atomic/pr80640.c b/gcc/testsuite/gcc.dg/atomic/pr80640.c
new file mode 100644
index 00000000000..fd17978a482
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/atomic/pr80640.c
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-pthread" } */
+/* { dg-require-effective-target pthread } */
+
+#include <pthread.h>
+
+static volatile int sem1;
+static volatile int sem2;
+
+static void *f(void *va)
+{
+ void **p = va;
+ if (*p) return *p;
+ sem1 = 1;
+ while (!sem2);
+ __atomic_thread_fence(__ATOMIC_ACQUIRE);
+ // GCC used to RTL-CSE this and the first load, causing 0 to be returned
+ return *p;
+}
+
+int main()
+{
+ void *p = 0;
+ pthread_t thr;
+ if (pthread_create(&thr, 0, f, &p))
+ return 2;
+ while (!sem1);
+ __atomic_thread_fence(__ATOMIC_ACQUIRE);
+ p = &p;
+ __atomic_thread_fence(__ATOMIC_RELEASE);
+ sem2 = 1;
+ pthread_join(thr, &p);
+ return !p;
+}
next prev parent reply other threads:[~2017-08-07 8:42 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-02 17:45 Alexander Monakov
2017-08-02 17:45 ` [PATCH 3/3] optabs: ensure atomic_load/stores have compiler barriers Alexander Monakov
2017-08-02 17:45 ` [PATCH 2/3] retire mem_signal_fence pattern Alexander Monakov
2017-08-28 12:39 ` [PING][PATCH " Alexander Monakov
[not found] ` <d738f0d1-e9d1-bfb2-e413-4d4b78370cdb@redhat.com>
2017-09-01 6:26 ` Alexander Monakov
2017-09-04 4:56 ` Jeff Law
2017-08-05 9:03 ` [PATCH 1/3] optabs: ensure mem_thread_fence is a compiler barrier Richard Sandiford
2017-08-07 8:42 ` Alexander Monakov [this message]
2017-08-14 20:46 ` Jeff Law
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=alpine.LNX.2.20.13.1708071135520.29517@monopod.intra.ispras.ru \
--to=amonakov@ispras.ru \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--cc=richard.sandiford@linaro.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).