diff mbox

bug wrt gcc 4.7 and the C++11 memory model

Message ID 5040EB8E.6090403@redhat.com
State New
Headers show

Commit Message

Richard Henderson Aug. 31, 2012, 4:51 p.m. UTC 
On 2012-08-31 05:48, Andrew MacLeod wrote:
> On 08/30/2012 07:04 PM, Richard Henderson wrote:
>> Actually, we already have a memory barrier feature in rtl:
>>
>>    ALIAS_SET_MEMORY_BARRIER
>>
>> but since we already set that in get_builtin_sync_mem, we'll need
>> to figure out why that's no longer working.
>>
>> As far as I can tell from reading alias.c, we should already be
>> indicating that such memories alias...
>>
>> What was the testcase again?  I seem to have lost the top of the thread...
>>
>>
> 
> #include <atomic>
> using namespace std;
> 
> atomic_uint a_8;
> int32_t g_70;
> int32_t g_141;
> 
> int main (int, char *[]) {
>    a_8.load () & a_8.load ();
>    g_141 = g_70 != 0;
> }
> 
> or equivalently
> 
> int a_8;
> int g_70;
> int g_141;
> 
> int main ()
> {
>    __atomic_load_n (&a_8, __ATOMIC_SEQ_CST) & __atomic_load_n (&a_8, __ATOMIC_SEQ_CST);
>    g_141 = g_70 != 0;
> }

Fixed as follows.  This seems to be the only entry point in alias.c that didn't take ALIAS_SET_MEMORY_BARRIER into account.  Getting the above into a test case is tricky, and I havn't figured out how to match it.

Full testing still underway, but I expect it to pass.


r~
* alias.c (read_dependence): Account for ALIAS_SET_MEMORY_BARRIER.

Comments

Steven Bosscher Aug. 31, 2012, 6:24 p.m. UTC | #1 
On Fri, Aug 31, 2012 at 6:51 PM, Richard Henderson <rth@redhat.com> wrote:
> Fixed as follows.  This seems to be the only entry point in alias.c that didn't take ALIAS_SET_MEMORY_BARRIER into account.  Getting the above into a test case is tricky, and I havn't figured out how to match it.
>
> Full testing still underway, but I expect it to pass.

Any reason to not add the (mem:BLK (scratch)) special case as well?

Ciao!
Steven
Steven Bosscher Aug. 31, 2012, 6:26 p.m. UTC | #2 
> Full testing still underway, but I expect it to pass.

Can you also update the comment before the function, please? It now reads:

/* Read dependence: X is read after read in MEM takes place.  There can
   only be a dependence here if both reads are volatile.  */

But that's no longer true after your patch.

Ciao!
Steven
diff mbox

Patch

diff --git a/gcc/alias.c b/gcc/alias.c
index e9d701f..5848e75 100644
--- a/gcc/alias.c
+++ b/gcc/alias.c
@@ -2132,7 +2132,12 @@  memrefs_conflict_p (int xsize, rtx x, int ysize, rtx y, HOST_WIDE_INT c)
 int
 read_dependence (const_rtx mem, const_rtx x)
 {
-  return MEM_VOLATILE_P (x) && MEM_VOLATILE_P (mem);
+  if (MEM_VOLATILE_P (x) && MEM_VOLATILE_P (mem))
+    return true;
+  if (MEM_ALIAS_SET (x) == ALIAS_SET_MEMORY_BARRIER
+      || MEM_ALIAS_SET (mem) == ALIAS_SET_MEMORY_BARRIER)
+    return true;
+  return false;
 }
 
 /* Returns nonzero if something about the mode or address format MEM1