[RX] Add support for atomic operations

[RX] Add support for atomic operations

[Oleg Endo]

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

Hi,

The attached patch adds some rudimentary support for atomic operations.
 On the original RX there is one truly atomic insn "xchg".  All other
operations have to implement atomicity in some other way.  One straight
forward option which is already being done on SH is to disable
interrupts for the duration of the atomic sequence.  This works for
single-core systems that run in privileged mode.  And that's what the
patch does.

OK for trunk?

Cheers,
Oleg

gcc/ChangeLog:
	* config/rx/rx-protos.h (is_interrupt_func, is_fast_interrupt_func):
	Forward declare.
	(rx_atomic_sequence): New class.
	* config/rx/rx.c (rx_print_operand): Use symbolic names for PSW bits.
	(is_interrupt_func, is_fast_interrupt_func): Make non-static and
	non-inline.
	(rx_atomic_sequence::rx_atomic_sequence,
	rx_atomic_sequence::~rx_atomic_sequence): New functions.
	* config/rx/rx.md (CTRLREG_PSW, CTRLREG_USP, CTRLREG_FPSW, CTRLREG_CPEN,
	CTRLREG_BPSW, CTRLREG_BPC, CTRLREG_ISP, CTRLREG_FINTV,
	CTRLREG_INTB): New constants.
	(FETCHOP): New code iterator.
	(fethcop_name, fetchop_name2): New iterator code attributes.
	(QIHI): New mode iterator.
	(atomic_exchange<mode>, atomic_exchangesi, xchg_mem<mode>,
	atomic_fetch_<fetchop_name>si, atomic_fetch_nandsi,
	atomic_<fetchop_name>_fetchsi, atomic_nand_fetchsi): New patterns.

[-- Attachment #2: gcc_rx_atomics.patch --]
[-- Type: text/x-patch, Size: 8516 bytes --]

Index: gcc/config/rx/rx-protos.h
===================================================================
--- gcc/config/rx/rx-protos.h	(revision 235992)
+++ gcc/config/rx/rx-protos.h	(working copy)
@@ -26,6 +26,28 @@
 extern void		rx_expand_prologue (void);
 extern int		rx_initial_elimination_offset (int, int);
 
+bool is_interrupt_func (const_tree decl);
+bool is_fast_interrupt_func (const_tree decl);
+
+/* rx_atomic_sequence is used to emit the header and footer
+   of an atomic sequence.  It's supposed to be used in a scope.
+   When constructed, it will emit the atomic sequence header insns.
+   When destructred (goes out of scope), it will emit the
+   corresponding atomic sequence footer insns.  */
+class rx_atomic_sequence
+{
+public:
+  rx_atomic_sequence (const_tree fun_decl);
+  ~rx_atomic_sequence (void);
+
+private:
+  rx_atomic_sequence (void);
+  rx_atomic_sequence (const rx_atomic_sequence&);
+  rx_atomic_sequence& operator = (const rx_atomic_sequence&);
+
+  rtx m_prev_psw_reg;
+};
+
 #ifdef RTX_CODE
 extern int		rx_adjust_insn_length (rtx_insn *, int);
 extern int 		rx_align_for_label (rtx, int);
Index: gcc/config/rx/rx.c
===================================================================
--- gcc/config/rx/rx.c	(revision 235992)
+++ gcc/config/rx/rx.c	(working copy)
@@ -630,15 +630,15 @@
       gcc_assert (CONST_INT_P (op));
       switch (INTVAL (op))
 	{
-	case 0:   fprintf (file, "psw"); break;
-	case 2:   fprintf (file, "usp"); break;
-	case 3:   fprintf (file, "fpsw"); break;
-	case 4:   fprintf (file, "cpen"); break;
-	case 8:   fprintf (file, "bpsw"); break;
-	case 9:   fprintf (file, "bpc"); break;
-	case 0xa: fprintf (file, "isp"); break;
-	case 0xb: fprintf (file, "fintv"); break;
-	case 0xc: fprintf (file, "intb"); break;
+	case CTRLREG_PSW:   fprintf (file, "psw"); break;
+	case CTRLREG_USP:   fprintf (file, "usp"); break;
+	case CTRLREG_FPSW:  fprintf (file, "fpsw"); break;
+	case CTRLREG_CPEN:  fprintf (file, "cpen"); break;
+	case CTRLREG_BPSW:  fprintf (file, "bpsw"); break;
+	case CTRLREG_BPC:   fprintf (file, "bpc"); break;
+	case CTRLREG_ISP:   fprintf (file, "isp"); break;
+	case CTRLREG_FINTV: fprintf (file, "fintv"); break;
+	case CTRLREG_INTB:  fprintf (file, "intb"); break;
 	default:
 	  warning (0, "unrecognized control register number: %d - using 'psw'",
 		   (int) INTVAL (op));
@@ -1216,7 +1216,7 @@
 
 /* Returns true if the provided function has the "fast_interrupt" attribute.  */
 
-static inline bool
+bool
 is_fast_interrupt_func (const_tree decl)
 {
   return has_func_attr (decl, "fast_interrupt");
@@ -1224,7 +1224,7 @@
 
 /* Returns true if the provided function has the "interrupt" attribute.  */
 
-static inline bool
+bool
 is_interrupt_func (const_tree decl)
 {
   return has_func_attr (decl, "interrupt");
@@ -3409,6 +3409,29 @@
   return TARGET_ENABLE_LRA;
 }
 
+rx_atomic_sequence::rx_atomic_sequence (const_tree fun_decl)
+{
+  if (is_fast_interrupt_func (fun_decl) || is_interrupt_func (fun_decl))
+    {
+      /* If we are inside an interrupt handler, assume that interrupts are
+	 off -- which is the default hardware behavior.  In this case, there
+	 is no need to disable the interrupts.  */
+      m_prev_psw_reg = NULL;
+    }
+  else
+    {
+      m_prev_psw_reg = gen_reg_rtx (SImode);
+      emit_insn (gen_mvfc (m_prev_psw_reg, GEN_INT (CTRLREG_PSW)));
+      emit_insn (gen_clrpsw (GEN_INT ('I')));
+    }
+}
+
+rx_atomic_sequence::~rx_atomic_sequence (void)
+{
+  if (m_prev_psw_reg != NULL)
+    emit_insn (gen_mvtc (GEN_INT (CTRLREG_PSW), m_prev_psw_reg));
+}
+
 \f
 #undef  TARGET_NARROW_VOLATILE_BITFIELD
 #define TARGET_NARROW_VOLATILE_BITFIELD		rx_narrow_volatile_bitfield
Index: gcc/config/rx/rx.md
===================================================================
--- gcc/config/rx/rx.md	(revision 235992)
+++ gcc/config/rx/rx.md	(working copy)
@@ -75,6 +75,16 @@
    (UNSPEC_BUILTIN_WAIT	   51)
 
    (UNSPEC_PID_ADDR	   52)
+
+   (CTRLREG_PSW		    0)
+   (CTRLREG_USP		    2)
+   (CTRLREG_FPSW	    3)
+   (CTRLREG_CPEN	    4)
+   (CTRLREG_BPSW	    8)
+   (CTRLREG_BPC		    9)
+   (CTRLREG_ISP		   10)
+   (CTRLREG_FINTV	   11)
+   (CTRLREG_INTB	   12)
   ]
 )
 
@@ -2145,8 +2155,18 @@
     FAIL;
 })
 \f
-;; Atomic exchange operation.
+;; Atomic operations.
 
+(define_code_iterator FETCHOP [plus minus ior xor and])
+
+(define_code_attr fetchop_name
+  [(plus "add") (minus "sub") (ior "or") (xor "xor") (and "and")])
+
+(define_code_attr fetchop_name2
+  [(plus "add") (minus "sub") (ior "ior") (xor "xor") (and "and")])
+
+(define_mode_iterator QIHI [QI HI])
+
 (define_insn "sync_lock_test_and_setsi"
   [(set (match_operand:SI 0 "register_operand"   "=r,r")
 	(match_operand:SI 1 "rx_compare_operand" "=r,Q"))
@@ -2157,6 +2177,126 @@
   [(set_attr "length" "3,6")
    (set_attr "timings" "22")]
 )
+
+(define_expand "atomic_exchange<mode>"
+  [(match_operand:QIHI 0 "register_operand")		;; oldval output
+   (match_operand:QIHI 1 "rx_restricted_mem_operand")	;; memory
+   (match_operand:QIHI 2 "register_operand")		;; newval input
+   (match_operand:QIHI 3 "const_int_operand")]		;; memory model
+  ""
+{
+  emit_insn (gen_xchg_mem<mode> (operands[0], operands[1], operands[2]));
+  DONE;
+})
+
+(define_expand "atomic_exchangesi"
+  [(match_operand:SI 0 "register_operand")		;; oldval output
+   (match_operand:SI 1 "rx_restricted_mem_operand")	;; memory
+   (match_operand:SI 2 "register_operand")		;; newval input
+   (match_operand:SI 3 "const_int_operand")]		;; memory model
+  ""
+{
+  emit_insn (gen_sync_lock_test_and_setsi (operands[0], operands[1],
+					   operands[2]));
+  DONE;
+})
+
+(define_insn "xchg_mem<mode>"
+  [(set (match_operand:QIHI 0 "register_operand"   "=r")
+	(match_operand:QIHI 1 "rx_compare_operand" "=Q"))
+   (set (match_dup 1)
+	(match_operand:QIHI 2 "register_operand"    "0"))]
+  ""
+  "xchg\t%1, %0"
+  [(set_attr "length" "6")
+   (set_attr "timings" "22")]
+)
+
+;; read - modify - write - return old value
+(define_expand "atomic_fetch_<fetchop_name>si"
+  [(set (match_operand:SI 0 "register_operand")
+	(match_operand:SI 1 "memory_operand"))
+   (set (match_dup 1)
+	(FETCHOP:SI (match_dup 1) (match_operand:SI 2 "rx_source_operand")))
+   (match_operand:SI 3 "const_int_operand")]		;; memory model
+  ""
+{
+  {
+    rx_atomic_sequence seq (current_function_decl);
+
+    emit_move_insn (operands[0], operands[1]);
+
+    rtx tmp = gen_reg_rtx (SImode);
+    emit_insn (gen_<fetchop_name2>si3 (tmp, operands[0], operands[2]));
+
+    emit_move_insn (operands[1], tmp);
+  }
+  DONE;
+})
+
+(define_expand "atomic_fetch_nandsi"
+  [(set (match_operand:SI 0 "register_operand")
+	(match_operand:SI 1 "memory_operand"))
+   (set (match_dup 1)
+	(not:SI (and:SI (match_dup 1)
+			(match_operand:SI 2 "rx_source_operand"))))
+   (match_operand:SI 3 "const_int_operand")]		;; memory model
+  ""
+{
+  {
+    rx_atomic_sequence seq (current_function_decl);
+
+    emit_move_insn (operands[0], operands[1]);
+
+    rtx tmp = gen_reg_rtx (SImode);
+    emit_insn (gen_andsi3 (tmp, operands[0], operands[2]));
+    emit_insn (gen_one_cmplsi2 (tmp, tmp));
+
+    emit_move_insn (operands[1], tmp);
+  }
+  DONE;
+})
+
+;; read - modify - write - return new value
+(define_expand "atomic_<fetchop_name>_fetchsi"
+  [(set (match_operand:SI 0 "register_operand")
+	(FETCHOP:SI (match_operand:SI 1 "rx_restricted_mem_operand")
+		    (match_operand:SI 2 "register_operand")))
+   (set (match_dup 1)
+	(FETCHOP:SI (match_dup 1) (match_dup 2)))
+   (match_operand:SI 3 "const_int_operand")]		;; memory model
+  ""
+{
+  {
+    rx_atomic_sequence seq (current_function_decl);
+
+    emit_move_insn (operands[0], operands[2]);
+    emit_insn (gen_<fetchop_name2>si3 (operands[0], operands[0], operands[1]));
+    emit_move_insn (operands[1], operands[0]);
+  }
+  DONE;
+})
+
+(define_expand "atomic_nand_fetchsi"
+  [(set (match_operand:SI 0 "register_operand")
+	(not:SI (and:SI (match_operand:SI 1 "rx_restricted_mem_operand")
+			(match_operand:SI 2 "register_operand"))))
+   (set (match_dup 1)
+	(not:SI (and:SI (match_dup 1) (match_dup 2))))
+   (match_operand:SI 3 "const_int_operand")]		;; memory model
+  ""
+{
+  {
+    rx_atomic_sequence seq (current_function_decl);
+
+    emit_move_insn (operands[0], operands[2]);
+    emit_insn (gen_andsi3 (operands[0], operands[0], operands[1]));
+    emit_insn (gen_one_cmplsi2 (operands[0], operands[0]));
+    emit_move_insn (operands[1], operands[0]);
+  }
+  DONE;
+});
+
 \f
 ;; Block move functions.
 

Re: [RX] Add support for atomic operations

[Nick Clifton]

Hi Oleg,

> gcc/ChangeLog:
> 	* config/rx/rx-protos.h (is_interrupt_func, is_fast_interrupt_func):
> 	Forward declare.
> 	(rx_atomic_sequence): New class.
> 	* config/rx/rx.c (rx_print_operand): Use symbolic names for PSW bits.
> 	(is_interrupt_func, is_fast_interrupt_func): Make non-static and
> 	non-inline.
> 	(rx_atomic_sequence::rx_atomic_sequence,
> 	rx_atomic_sequence::~rx_atomic_sequence): New functions.
> 	* config/rx/rx.md (CTRLREG_PSW, CTRLREG_USP, CTRLREG_FPSW, CTRLREG_CPEN,
> 	CTRLREG_BPSW, CTRLREG_BPC, CTRLREG_ISP, CTRLREG_FINTV,
> 	CTRLREG_INTB): New constants.
> 	(FETCHOP): New code iterator.
> 	(fethcop_name, fetchop_name2): New iterator code attributes.
> 	(QIHI): New mode iterator.
> 	(atomic_exchange<mode>, atomic_exchangesi, xchg_mem<mode>,
> 	atomic_fetch_<fetchop_name>si, atomic_fetch_nandsi,
> 	atomic_<fetchop_name>_fetchsi, atomic_nand_fetchsi): New patterns.

Approved - please apply.

Cheers
  Nick

Re: [RX] Add support for atomic operations

[Oleg Endo]

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

On Mon, 2016-05-09 at 15:18 +0100, Nick Clifton wrote:

> > gcc/ChangeLog:
> > 	* config/rx/rx-protos.h (is_interrupt_func,
> > is_fast_interrupt_func):
> > 	Forward declare.
> > 	(rx_atomic_sequence): New class.
> > 	* config/rx/rx.c (rx_print_operand): Use symbolic names for PSW
> > bits.
> > 	(is_interrupt_func, is_fast_interrupt_func): Make non-static
> > and
> > 	non-inline.
> > 	(rx_atomic_sequence::rx_atomic_sequence,
> > 	rx_atomic_sequence::~rx_atomic_sequence): New functions.
> > 	* config/rx/rx.md (CTRLREG_PSW, CTRLREG_USP, CTRLREG_FPSW,
> > CTRLREG_CPEN,
> > 	CTRLREG_BPSW, CTRLREG_BPC, CTRLREG_ISP, CTRLREG_FINTV,
> > 	CTRLREG_INTB): New constants.
> > 	(FETCHOP): New code iterator.
> > 	(fethcop_name, fetchop_name2): New iterator code attributes.
> > 	(QIHI): New mode iterator.
> > 	(atomic_exchange<mode>, atomic_exchangesi, xchg_mem<mode>,
> > 	atomic_fetch_<fetchop_name>si, atomic_fetch_nandsi,
> > 	atomic_<fetchop_name>_fetchsi, atomic_nand_fetchsi): New
> > patterns.
> 
> Approved - please apply.
> 

Sorry, but my original patch was buggy.  There are two problems:

First, when interrupts are re-enabled by restoring the PSW using the
"mvtc" insn after the atomic sequence, the CC_REG is clobbered.  It's
not entirely clear to me why leaving out the CC_REG clobber in "mvtc"
is of any benefit.  Instead of adding a new "mvtc" pattern, I've just
added the clobber to the existing one.  With that wrong code issues
around atomic sequences such as atomic decrement and test for zero are
fixed.

Second, the atomic_<fetchop_name>_fetchsi works only with commutative
operations because the memory operand and the register operand are
swapped in the expander.  Thus it produces wrong code for subtraction
operations.  The fix is to use a separate pattern for subtraction and
not twist the operands.

The attached patch fixes those issues.
OK for trunk?

Cheers,
Oleg

gcc/ChangeLog:
	* config/rx/rx.md (FETCHOP_NO_MINUS): New code iterator.
	(atomic_<fetchop_name>_fetchsi): Extract minus operator into ...
	(atomic_sub_fetchsi): ... this new pattern.
	(mvtc): Add CC_REG clobber.

[-- Attachment #2: gcc_rx_atomics_2.patch --]
[-- Type: text/x-patch, Size: 2728 bytes --]

Index: gcc/config/rx/rx.md
===================================================================
--- gcc/config/rx/rx.md	(revision 236761)
+++ gcc/config/rx/rx.md	(working copy)
@@ -2158,6 +2158,7 @@
 ;; Atomic operations.
 
 (define_code_iterator FETCHOP [plus minus ior xor and])
+(define_code_iterator FETCHOP_NO_MINUS [plus ior xor and])
 
 (define_code_attr fetchop_name
   [(plus "add") (minus "sub") (ior "or") (xor "xor") (and "and")])
@@ -2258,12 +2259,14 @@
 })
 
 ;; read - modify - write - return new value
+;; Because subtraction is not commutative we need to specify a different
+;; set of patterns for it.
 (define_expand "atomic_<fetchop_name>_fetchsi"
   [(set (match_operand:SI 0 "register_operand")
-	(FETCHOP:SI (match_operand:SI 1 "rx_restricted_mem_operand")
-		    (match_operand:SI 2 "register_operand")))
+	(FETCHOP_NO_MINUS:SI (match_operand:SI 1 "rx_restricted_mem_operand")
+			     (match_operand:SI 2 "register_operand")))
    (set (match_dup 1)
-	(FETCHOP:SI (match_dup 1) (match_dup 2)))
+	(FETCHOP_NO_MINUS:SI (match_dup 1) (match_dup 2)))
    (match_operand:SI 3 "const_int_operand")]		;; memory model
   ""
 {
@@ -2277,6 +2280,25 @@
   DONE;
 })
 
+(define_expand "atomic_sub_fetchsi"
+  [(set (match_operand:SI 0 "register_operand")
+	(minus:SI (match_operand:SI 1 "rx_restricted_mem_operand")
+		  (match_operand:SI 2 "rx_source_operand")))
+   (set (match_dup 1)
+	(minus:SI (match_dup 1) (match_dup 2)))
+   (match_operand:SI 3 "const_int_operand")]		;; memory model
+  ""
+{
+  {
+    rx_atomic_sequence seq (current_function_decl);
+
+    emit_move_insn (operands[0], operands[1]);
+    emit_insn (gen_subsi3 (operands[0], operands[0], operands[2]));
+    emit_move_insn (operands[1], operands[0]);
+  }
+  DONE;
+})
+
 (define_expand "atomic_nand_fetchsi"
   [(set (match_operand:SI 0 "register_operand")
 	(not:SI (and:SI (match_operand:SI 1 "rx_restricted_mem_operand")
@@ -2674,18 +2696,16 @@
 )
 
 ;; Move to control register
+;; This insn can be used in atomic sequences to restore the previous PSW
+;; and re-enable interrupts.  Because of that it always clobbers the CC_REG.
 (define_insn "mvtc"
   [(unspec_volatile:SI [(match_operand:SI 0 "immediate_operand" "i,i")
 	       (match_operand:SI 1 "nonmemory_operand" "r,i")]
-	      UNSPEC_BUILTIN_MVTC)]
+	      UNSPEC_BUILTIN_MVTC)
+   (clobber (reg:CC CC_REG))]
   ""
   "mvtc\t%1, %C0"
   [(set_attr "length" "3,7")]
-  ;; Ignore possible clobbering of the comparison flags in the
-  ;; PSW register.  This is a cc0 target so any cc0 setting
-  ;; instruction will always be paired with a cc0 user, without
-  ;; the possibility of this instruction being placed in between
-  ;; them.
 )
 
 ;; Move to interrupt priority level

Re: [RX] Add support for atomic operations

[Nick Clifton]

Hi Oleg,

> Sorry, but my original patch was buggy.  There are two problems:

Thanks for your diligence in checking the patch.

> The attached patch fixes those issues.
> OK for trunk?
> 
> Cheers,
> Oleg
> 
> gcc/ChangeLog:
> 	* config/rx/rx.md (FETCHOP_NO_MINUS): New code iterator.
> 	(atomic_<fetchop_name>_fetchsi): Extract minus operator into ...
> 	(atomic_sub_fetchsi): ... this new pattern.
> 	(mvtc): Add CC_REG clobber.

Approved - please apply.

Cheers

Re: [RX] Add support for atomic operations

[Oleg Endo]

On Tue, 2016-05-31 at 14:17 +0100, Nick Clifton wrote:

> > Sorry, but my original patch was buggy.  There are two problems:
> 
> Thanks for your diligence in checking the patch.

Just eating my own dogfood here ... :)

> Approved - please apply.

Committed as r236926.

Cheers,
Oleg