[gomp4] fix spinlock

[gomp4] fix spinlock

[Nathan Sidwell]

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

I've committed this to fix the spinlock problem Cesar fell over.  While there I 
added more checking on the worker dimension.

nathan

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

2015-08-06  Nathan Sidwell  <nathan@codesourcery.com>

	* config/nvptx/nvptx.c (nvptx_expand_lock_unlock): Create label.
	(nvptx_validate_dims): Check worker dimension is not too big.
	* config/nvptx/nvptx.mc (nvptx_spinlock): Take label.

Index: gcc/config/nvptx/nvptx.c
===================================================================
--- gcc/config/nvptx/nvptx.c	(revision 226605)
+++ gcc/config/nvptx/nvptx.c	(working copy)
@@ -3318,8 +3318,14 @@ nvptx_expand_lock_unlock (tree exp, bool
   if (!lock)
     emit_insn (barrier);
   if (lock)
-    pat = gen_nvptx_spinlock (mem, space,
-			      gen_reg_rtx (SImode), gen_reg_rtx (BImode));
+    {
+      rtx_code_label *label = gen_label_rtx ();
+
+      LABEL_NUSES (label)++;
+      pat = gen_nvptx_spinlock (mem, space,
+				gen_reg_rtx (SImode), gen_reg_rtx (BImode),
+				label);
+    }
   else
     pat = gen_nvptx_spinunlock (mem, space);
   emit_insn (pat);
@@ -3531,7 +3537,6 @@ nvptx_validate_dims (tree decl, tree dim
 {
   tree adims[GOMP_DIM_MAX];
   unsigned ix;
-  bool changed = false;
   tree pos = dims;
 
   for (ix = 0; ix != GOMP_DIM_MAX; ix++)
@@ -3541,6 +3546,7 @@ nvptx_validate_dims (tree decl, tree dim
     }
   /* Define vector size for known hardware.  */
 #define PTX_VECTOR_LENGTH 32
+#define PTX_WORKER_LENGTH 32
   /* If the worker size is not 1, the vector size must be 32.  If
      the vector size is not 1, it must be 32.  */
   if ((adims[GOMP_DIM_WORKER]
@@ -3562,6 +3568,17 @@ nvptx_validate_dims (tree decl, tree dim
 	}
     }
 
+  /* Check the num workers is not too large.  */
+  if (adims[GOMP_DIM_WORKER]
+      && TREE_INT_CST_LOW (adims[GOMP_DIM_WORKER]) > PTX_WORKER_LENGTH)
+    {
+      tree use = build_int_cst (integer_type_node, PTX_WORKER_LENGTH);
+      warning_at (DECL_SOURCE_LOCATION (decl), 0,
+		  "using num_workers (%E), ignoring %E",
+		  use, adims[GOMP_DIM_WORKER]);
+      adims[GOMP_DIM_WORKER] = use;
+    }
+
   /* Set defaults.  */
   for (ix = 0; ix != GOMP_DIM_MAX; ix++)
     if (!adims[ix])
Index: gcc/config/nvptx/nvptx.md
===================================================================
--- gcc/config/nvptx/nvptx.md	(revision 226605)
+++ gcc/config/nvptx/nvptx.md	(working copy)
@@ -1578,9 +1578,10 @@
 			(match_operand:SI 1 "const_int_operand" "i")]
 		       UNSPECV_UNLOCK)
       (match_operand:SI 2 "register_operand" "=R")
-      (match_operand:BI 3 "register_operand" "=R")])]
+      (match_operand:BI 3 "register_operand" "=R")
+      (label_ref (match_operand 4 "" ""))])]
    ""
-   "1:\\t.atom%R1.cas.b32 %2,%0,0,1;setp.ne.u32 %3,%2,0;@%3 bra.uni 1b;")
+   "%4:\\t.atom%R1.cas.b32 %2,%0,0,1;setp.ne.u32 %3,%2,0;@%3 bra.uni %4;")
 
 (define_insn "nvptx_spinunlock"
    [(unspec_volatile [(match_operand:SI 0 "memory_operand" "m")

Re: [gomp4] fix spinlock

[Cesar Philippidis]

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

On 08/06/2015 01:41 AM, Nathan Sidwell wrote:

> I've committed this to fix the spinlock problem Cesar fell over.  While
> there I added more checking on the worker dimension.

I hit a couple of more bugs with the spinlocks. First, the address space
argument to membar wasn't being handled properly. Second,
nvptx_spinunlock should probably be using atom.exch instead of atom.cas.
Finally, ptxas complains about the period prefix to the atom
instructions. This patch addresses these problems.

Is there a better way to allocate a scratch register for
nvptx_spinunlock, or is my solution ok as-is for gomp-4_0-branch?

Thanks,
Cesar




[-- Attachment #2: nvptx-atomics.diff --]
[-- Type: text/x-patch, Size: 1815 bytes --]

2015-08-06  Cesar Philippidis  <cesar@codesourcery.com>

	gcc/
	* config/nvptx/nvptx.c (nvptx_expand_lock_unlock): Pass an
	additional scratch register to gen_nvptx_spinlock.
	* config/nvptx/nvptx.md (nvptx_membar): Use %B for the address
	space operand.
	(nvptx_spinlock): Remove period prefix from atom.
	(nvptx_spinunlock): Take additional scratch register argument.
	Use atom.exch to update the lock.
	

diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 2013219..881aea4 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -3327,7 +3327,7 @@ nvptx_expand_lock_unlock (tree exp, bool lock)
 				label);
     }
   else
-    pat = gen_nvptx_spinunlock (mem, space);
+    pat = gen_nvptx_spinunlock (mem, space, gen_reg_rtx (SImode));
   emit_insn (pat);
   if (lock)
     emit_insn (barrier);
diff --git a/gcc/config/nvptx/nvptx.md b/gcc/config/nvptx/nvptx.md
index 8cd8300..fb88c72 100644
--- a/gcc/config/nvptx/nvptx.md
+++ b/gcc/config/nvptx/nvptx.md
@@ -1569,7 +1569,7 @@
   [(unspec_volatile [(match_operand:SI 0 "const_int_operand" "")]
 		    UNSPECV_MEMBAR)]
   ""
-  "membar%M0;")
+  "membar%B0;")
 
 ;; spinlock and unlock
 (define_insn "nvptx_spinlock"
@@ -1581,11 +1581,12 @@
       (match_operand:BI 3 "register_operand" "=R")
       (label_ref (match_operand 4 "" ""))])]
    ""
-   "%4:\\t.atom%R1.cas.b32 %2,%0,0,1;setp.ne.u32 %3,%2,0;@%3 bra.uni %4;")
+   "%4:\\tatom%R1.cas.b32 %2,%0,0,1;setp.ne.u32 %3,%2,0;@%3 bra.uni %4;")
 
 (define_insn "nvptx_spinunlock"
    [(unspec_volatile [(match_operand:SI 0 "memory_operand" "m")
 		      (match_operand:SI 1 "const_int_operand" "i")]
-		      UNSPECV_UNLOCK)]
+		      UNSPECV_UNLOCK)
+    (match_operand:SI 2 "register_operand" "=R")]
    ""
-   ".atom%R1.cas.b32 %0,1,0;")
+   "atom%R1.exch.b32 %2,%0,0;")

Re: [gomp4] fix spinlock

[Nathan Sidwell]

On 08/06/15 17:56, Cesar Philippidis wrote:
> On 08/06/2015 01:41 AM, Nathan Sidwell wrote:
>
>> I've committed this to fix the spinlock problem Cesar fell over.  While
>> there I added more checking on the worker dimension.
>
> I hit a couple of more bugs with the spinlocks. First, the address space
> argument to membar wasn't being handled properly.
OK

> Second,
> nvptx_spinunlock should probably be using atom.exch instead of atom.cas.

I'm ambivalent about this.  I chese cas precisely to dodge the issue of needing 
a temp reg.

> Finally, ptxas complains about the period prefix to the atom
> instructions. This patch addresses these problems.

ok


> Is there a better way to allocate a scratch register for
> nvptx_spinunlock, or is my solution ok as-is for gomp-4_0-branch?

Allocation is ok.  I guess really this should be a SET and we could even name it 
correctly as an atomic exchange, but that's probably more than needed right no.

patch is ok,thanks