On Fri, Jun 27, 2014 at 10:04 PM, Richard Henderson <r...@redhat.com> wrote:
> On 06/27/2014 10:04 AM, Uros Bizjak wrote:
>> This happened due to the way stores to QImode and HImode locations are
>> implemented on non-BWX targets. The sequence reads full word, does its
>> magic to the part and stores the full word with changed part back to
>> the memory. However - the scheduler mixed two sequences, violating the
>> atomicity of RMW sequence.
>
> Except that addresses with AND in them (ldq_u, stq_u) are already supposed to
> be memory barriers.  So you shouldn't need to add such to them again.
>
> I think something else has gone wrong somewhere.

There is am early shortcut for MEM_READOLNY_P in true_dependence_1.

In this case,

(insn 15 13 18 2 (set (reg/f:DI 78 [ b ])
        (mem/u/f/c:DI (reg/f:DI 79) [2 b+0 S8 A64])) rmw.c:7 226 {*movdi}
     (expr_list:REG_DEAD (reg/f:DI 79)
        (nil)))

is free to be scheduled before

(insn 13 12 15 2 (set (mem:DI (and:DI (plus:DI (reg/f:DI 72 [ a ])
                    (const_int 1 [0x1]))
                (const_int -8 [0xfffffffffffffff8])) [0  S8 A64])
        (reg:DI 77)) rmw.c:6 226 {*movdi}
     (expr_list:REG_DEAD (reg:DI 77)
        (expr_list:REG_DEAD (reg/f:DI 72 [ a ])
            (nil))))

Attached RFC patch also fixes the testcase, trivially preventing
scheduling of (insn 13) before (insn 15).

Uros.
Index: alias.c
===================================================================
--- alias.c     (revision 212063)
+++ alias.c     (working copy)
@@ -2468,10 +2468,19 @@
       || MEM_ALIAS_SET (mem) == ALIAS_SET_MEMORY_BARRIER)
     return 1;
 
+  if (! mem_addr)
+    {
+      mem_addr = XEXP (mem, 0);
+      if (mem_mode == VOIDmode)
+       mem_mode = GET_MODE (mem);
+    }
+
   /* Read-only memory is by definition never modified, and therefore can't
      conflict with anything.  We don't expect to find read-only set on MEM,
      but stupid user tricks can produce them, so don't die.  */
   if (MEM_READONLY_P (x))
+    if (GET_CODE (mem_addr) == AND)
+      return 1;
     return 0;
 
   /* If we have MEMs referring to different address spaces (which can
@@ -2480,13 +2489,6 @@
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
     return 1;
 
-  if (! mem_addr)
-    {
-      mem_addr = XEXP (mem, 0);
-      if (mem_mode == VOIDmode)
-       mem_mode = GET_MODE (mem);
-    }
-
   if (! x_addr)
     {
       x_addr = XEXP (x, 0);

Reply via email to