public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix for PR55492 : __atomic_load doesn't match ACQUIRE memory model
@ 2012-11-29 15:42 Yvan Roux
  2012-11-29 17:01 ` Andrew MacLeod
  2012-11-30  9:37 ` Marcus Shawcroft
  0 siblings, 2 replies; 5+ messages in thread
From: Yvan Roux @ 2012-11-29 15:42 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

Hi,

on ARMv7, the code generated for the __atomic_load builtins in the
__ATOMIC_ACQUIRE memory model, puts a memory barrier before the load, whereas
the semantic of the acquire memory model implies a barrier after.

The issue seems to be in expand_atomic_load which puts a memory fence before
the load in any memory model. The attached patch fixes the problem.

Thanks,
Yvan

[-- Attachment #2: Fix-load-acquire.patch --]
[-- Type: application/octet-stream, Size: 883 bytes --]

2012-11-29  Yvan Roux  <yvan.roux@st.com>

	PR target/55942
	* optabs.c (expand_atomic_load): Condition the memory fence to the
	the right memory model.

diff --git a/gcc/optabs.c b/gcc/optabs.c
index 353727f..14956be 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -7478,12 +7478,13 @@ expand_atomic_load (rtx target, rtx mem, enum memmodel model)
     target = gen_reg_rtx (mode);
 
   /* Emit the appropriate barrier before the load.  */
-  expand_mem_thread_fence (model);
+  if (model == MEMMODEL_SEQ_CST)
+    expand_mem_thread_fence (model);
 
   emit_move_insn (target, mem);
 
-  /* For SEQ_CST, also emit a barrier after the load.  */
-  if (model == MEMMODEL_SEQ_CST)
+  /* For SEQ_CST or ACQUIRE, also emit a barrier after the load.  */
+  if (model == MEMMODEL_SEQ_CST || model == MEMMODEL_ACQUIRE)
     expand_mem_thread_fence (model);
 
   return target;
-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix for PR55492 : __atomic_load doesn't match ACQUIRE memory model
  2012-11-29 15:42 [PATCH] Fix for PR55492 : __atomic_load doesn't match ACQUIRE memory model Yvan Roux
@ 2012-11-29 17:01 ` Andrew MacLeod
  2012-11-29 17:10   ` Yvan Roux
  2012-11-30  9:37 ` Marcus Shawcroft
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew MacLeod @ 2012-11-29 17:01 UTC (permalink / raw)
  To: Yvan Roux; +Cc: gcc-patches

On 11/29/2012 10:42 AM, Yvan Roux wrote:
> Hi,
>
> on ARMv7, the code generated for the __atomic_load builtins in the
> __ATOMIC_ACQUIRE memory model, puts a memory barrier before the load, whereas
> the semantic of the acquire memory model implies a barrier after.
>
> The issue seems to be in expand_atomic_load which puts a memory fence before
> the load in any memory model. The attached patch fixes the problem.
>
Patch is fine.

If you can confirm you have a copyright assignment on file I can take 
care of checking it in.

Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix for PR55492 : __atomic_load doesn't match ACQUIRE memory model
  2012-11-29 17:01 ` Andrew MacLeod
@ 2012-11-29 17:10   ` Yvan Roux
  0 siblings, 0 replies; 5+ messages in thread
From: Yvan Roux @ 2012-11-29 17:10 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches

> If you can confirm you have a copyright assignment on file I can take care
> of checking it in.

Yes, I am covered by the copyright assignement RT 211150 between
STMicroelectronics and FSF.

Thanks,
Yvan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix for PR55492 : __atomic_load doesn't match ACQUIRE memory model
  2012-11-29 15:42 [PATCH] Fix for PR55492 : __atomic_load doesn't match ACQUIRE memory model Yvan Roux
  2012-11-29 17:01 ` Andrew MacLeod
@ 2012-11-30  9:37 ` Marcus Shawcroft
  2012-11-30 10:15   ` Yvan Roux
  1 sibling, 1 reply; 5+ messages in thread
From: Marcus Shawcroft @ 2012-11-30  9:37 UTC (permalink / raw)
  To: gcc-patches

On 29/11/12 15:42, Yvan Roux wrote:
> Hi,
>
> on ARMv7, the code generated for the __atomic_load builtins in the
> __ATOMIC_ACQUIRE memory model, puts a memory barrier before the load, whereas
> the semantic of the acquire memory model implies a barrier after.
>
> The issue seems to be in expand_atomic_load which puts a memory fence before
> the load in any memory model. The attached patch fixes the problem.
>
> Thanks,
> Yvan
>

The same issue exists in the 4.7 branch.

Can the fix be back ported?

/Marcus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix for PR55492 : __atomic_load doesn't match ACQUIRE memory model
  2012-11-30  9:37 ` Marcus Shawcroft
@ 2012-11-30 10:15   ` Yvan Roux
  0 siblings, 0 replies; 5+ messages in thread
From: Yvan Roux @ 2012-11-30 10:15 UTC (permalink / raw)
  To: Marcus Shawcroft; +Cc: gcc-patches

> Can the fix be back ported?

Yes, the back port on 4.7 is straightforward, it just needs to be commited.

Yvan

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-11-30  9:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-29 15:42 [PATCH] Fix for PR55492 : __atomic_load doesn't match ACQUIRE memory model Yvan Roux
2012-11-29 17:01 ` Andrew MacLeod
2012-11-29 17:10   ` Yvan Roux
2012-11-30  9:37 ` Marcus Shawcroft
2012-11-30 10:15   ` Yvan Roux

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).