diff mbox

PR67401: Fix wrong code generated by expand_atomic_compare_and_swap

Message ID 55F2F01C.5@bell.net
State New
Headers show

Commit Message

John David Anglin Sept. 11, 2015, 3:15 p.m. UTC
On 2015-09-11 4:15 AM, Bernd Schmidt wrote:
> On 09/11/2015 01:21 AM, John David Anglin wrote:
>> As noted in the PR, expand_atomic_compare_and_swap can generate wrong 
>> code when libcalls are emitted
>> for the sync_compare_and_swap and the result comparison test. This is 
>> fixed by emitting a move insn to copy
>> the result rtx of the sync_compare_and_swap libcall to target_oval 
>> instead of directly assigning it.
> Could you provide relevant parts of the rtl dumps or (preferrably) the 
> patch you are using to enable the libcall?

This can be duplicated with a cross to hppa-unknown-linux-gnu with the 
following change to enable the libcall:


(call_insn 48 47 49 (parallel [
             (set (reg:DI 28 %r28)
                 (call (mem:SI (symbol_ref/v:SI 
("@__sync_val_compare_and_swap_8") [flags 0x41]) [0  S4 A32])
                     (const_int 64 [0x40])))
             (clobber (reg:SI 1 %r1))
             (clobber (reg:SI 2 %r2))
             (use (const_int 0 [0]))
         ]) xxx.c:6 -1
      (expr_list:REG_EH_REGION (const_int -2147483648 [0xffffffff80000000])
         (nil))
     (expr_list (use (reg:DI 23 %r23))
         (expr_list (use (reg:SI 26 %r26))
             (expr_list (use (mem:DI (plus:SI (reg/f:SI 93 
virtual-outgoing-args)
                             (const_int -24 [0xffffffffffffffe8])) [0  
S8 A64]))
                 (nil)))))

(insn 49 48 50 (set (reg:SI 128)
         (const_int 1 [0x1])) xxx.c:6 -1
      (nil))

(insn 50 49 51 (set (reg:DI 23 %r23)
         (reg:DI 123)) xxx.c:6 -1
      (nil))

(insn 51 50 52 (set (reg:DI 25 %r25)
         (reg:DI 28 %r28)) xxx.c:6 -1
      (nil))

(call_insn/u 52 51 53 (parallel [
             (set (reg:SI 28 %r28)
                 (call (mem:SI (symbol_ref/v:SI ("@__ucmpdi2") [flags 
0x41]) [0  S4 A32])
                     (const_int 64 [0x40])))
             (clobber (reg:SI 1 %r1))
             (clobber (reg:SI 2 %r2))
             (use (const_int 0 [0]))
         ]) xxx.c:6 -1
      (expr_list:REG_EH_REGION (const_int -2147483648 [0xffffffff80000000])
         (nil))
     (expr_list (use (reg:DI 23 %r23))
         (expr_list (use (reg:DI 25 %r25))
             (nil))))

(jump_insn 53 52 54 (set (pc)
         (if_then_else (eq (reg:SI 28 %r28)
                 (const_int 1 [0x1]))
             (label_ref 55)
             (pc))) xxx.c:6 -1
      (nil))

(insn 54 53 55 (set (reg:SI 128)
         (const_int 0 [0])) xxx.c:6 -1
      (nil))

(code_label 55 54 56 3 "" [0 uses])

(jump_insn 56 55 57 (set (pc)
         (if_then_else (ne (reg:SI 128)
                 (const_int 0 [0]))
             (label_ref 58)
             (pc))) xxx.c:6 -1
      (nil))

(insn 57 56 58 (set (mem:DI (reg:SI 122) [0  S8 A64])
         (reg:DI 28 %r28)) xxx.c:6 -1
      (nil))

(code_label 58 57 59 4 "" [0 uses])

(insn 59 58 0 (set (reg:SI 102)
         (reg:SI 128)) xxx.c:6 -1
      (nil))

;; if (_17 != 0)

(jump_insn 60 59 0 (set (pc)
         (if_then_else (ne (reg:SI 102)
                 (const_int 0 [0]))
             (label_ref 0)
             (pc))) xxx.c:6 -1
      (nil))

The value in reg:DI 28 returned by the __sync_val_compare_and_swap_8 is 
clobbered by the
the call to __ucmpdi2.  It is needed in insn 57.

Dave

Comments

Bernd Schmidt Sept. 14, 2015, 8:43 a.m. UTC | #1
On 09/11/2015 05:15 PM, John David Anglin wrote:
> On 2015-09-11 4:15 AM, Bernd Schmidt wrote:
>> On 09/11/2015 01:21 AM, John David Anglin wrote:
>>> As noted in the PR, expand_atomic_compare_and_swap can generate wrong
>>> code when libcalls are emitted
>>> for the sync_compare_and_swap and the result comparison test. This is
>>> fixed by emitting a move insn to copy
>>> the result rtx of the sync_compare_and_swap libcall to target_oval
>>> instead of directly assigning it.
>> Could you provide relevant parts of the rtl dumps or (preferrably) the
>> patch you are using to enable the libcall?
>
> This can be duplicated with a cross to hppa-unknown-linux-gnu with the
> following change to enable the libcall:

Ok, thanks, This patch is ok.


Bernd
diff mbox

Patch

Index: config/pa/pa.c
===================================================================
--- config/pa/pa.c      (revision 227689)
+++ config/pa/pa.c      (working copy)
@@ -5737,7 +5737,7 @@ 
      }

    if (TARGET_SYNC_LIBCALL)
-    init_sync_libfuncs (UNITS_PER_WORD);
+    init_sync_libfuncs (8);
  }

The relevant rtl from .expand is: