diff mbox

[gomp4] remove goacc locking

Message ID 87612rronb.fsf@kepler.schwinge.homeip.net
State New
Headers show

Commit Message

Thomas Schwinge Oct. 1, 2015, 8:14 a.m. UTC 
Hi Nathan!

On Mon, 28 Sep 2015 11:56:09 -0400, Nathan Sidwell <nathan@acm.org> wrote:
> I've committed this to remove the now no longer needed lock and unlock builtins 
> and related infrastructure.

If I understand correctly, it is an implementation detail of the nvptx
offloading implementation that it doesn't require such locking
primitives, but such locking may still be required for other (future)
offloading implementations, at which point something like the following
would have to be introduced again:

> 	* target.def (GOACC_LOCK): Delete hook.
> 	* doc/tm.texi.in (TARGET_GOACC_LOCK): Delete.
> 	* doc/tm.texi: Rebuilt.
> 	* targhooks.h (default_goacc_lock): Delete.
> 	* internal-fn.def (GOACC_LOCK,  GOACC_UNLOCK, GOACC_LOCK_INIT): Delete.
> 	* internal-fn.c (expand_GOACC_LOCK, expand_GOACC_UNLOCK,
> 	expand_GOACC_LOCK_INIT): Delete.
> 	* omp-low.c (lower_oacc_reductions): Remove locking.
> 	(execute_oacc_transform): Remove lock transforming.
> 	(default_goacc_lock): Delete.

(Of course, I agree that it doesn't make sense to try to maintain such a
locking implementation, if now/currently there isn't any user of it.)

> 	* config/nvptx/nvptx-protos.h (nvptx_expand_oacc_lock): Delete.
> 	* config/nvptx/nvptx.md (oacc_lock, oacc_unlock, oacc_lock_init):
> 	Delete.
> 	(nvptx_spin_lock, nvptx_spin_reset): Delete.
> 	* config/nvptx/nvptx.c (LOCK_GLOBAL, LOCK_SHARED, LOCK_MAX): Delete.
> 	(lock_names, lock_space, lock_level, lock_used): Delete.
> 	(force_global_locks): Delete.
> 	(nvptx_option_override): Do not initialize lock syms.
> 	(nvptx_expand_oacc_lock): Delete.
> 	(nvptx_file_end): Do not finalize locks.
> 	(TARGET_GOACC_LOCK): Delete.

> --- config/nvptx/nvptx.c	(revision 228200)
> +++ config/nvptx/nvptx.c	(working copy)

> @@ -4930,9 +4864,6 @@ nvptx_use_anchors_for_symbol (const_rtx
>  #undef TARGET_GOACC_FORK_JOIN
>  #define TARGET_GOACC_FORK_JOIN nvptx_xform_fork_join
>  
> -#undef TARGET_GOACC_LOCK
> -#define TARGET_GOACC_LOCK nvptx_xform_lock
> -
>  #undef TARGET_GOACC_REDUCTION
>  #define TARGET_GOACC_REDUCTION nvptx_goacc_reduction

    [...]/source-gcc/gcc/config/nvptx/nvptx.c:4328:1: warning: 'bool nvptx_xform_lock(gcall*, const int*, unsigned int)' defined but not used [-Wunused-function]+}
     nvptx_xform_lock (gcall *ARG_UNUSED (call),+}
     ^+}

Committed to gomp-4_0-branch in r228321:

commit a65ecefb6320407b221297880e9c25e1b7b9a430
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Thu Oct 1 08:12:01 2015 +0000

    Address -Wunused-function diagnostic
    
    	gcc/
    	* config/nvptx/nvptx.c (nvptx_xform_lock): Remove function.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@228321 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog.gomp       |    4 ++++
 gcc/config/nvptx/nvptx.c |    9 ---------
 2 files changed, 4 insertions(+), 9 deletions(-)



Grüße,
 Thomas

Comments

Nathan Sidwell Oct. 1, 2015, 12:28 p.m. UTC | #1 
On 10/01/15 04:14, Thomas Schwinge wrote:
> Hi Nathan!
>
> On Mon, 28 Sep 2015 11:56:09 -0400, Nathan Sidwell <nathan@acm.org> wrote:
>> I've committed this to remove the now no longer needed lock and unlock builtins
>> and related infrastructure.
>
> If I understand correctly, it is an implementation detail of the nvptx
> offloading implementation that it doesn't require such locking
> primitives, but such locking may still be required for other (future)
> offloading implementations, at which point something like the following
> would have to be introduced again:

I thought about that.  Other implementations can also use the lockless idiom. 
There's always going to be a cmp&swap atomic, otherwise the HW is terribly 
designed.  lock/unlock doesn't work between PTX warps, as we've discovered, and 
I think the same will probably be true of HSA, as that's roughly the same 
conceptually.  If it turns out I'm wrong, these bits can always be resurrected then.

The reason I originally went with lock/unlock is that it was conceptually 
simpler.  The lockless scheme is quite straight forwards, once one understands 
the underlying concept.

nathan
diff mbox

Patch

diff --git gcc/ChangeLog.gomp gcc/ChangeLog.gomp
index c4033e0..3a9e01d 100644
--- gcc/ChangeLog.gomp
+++ gcc/ChangeLog.gomp
@@ -1,3 +1,7 @@ 
+2015-10-01  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* config/nvptx/nvptx.c (nvptx_xform_lock): Remove function.
+
 2015-09-30  Thomas Schwinge  <thomas@codesourcery.com>
 
 	* tree-cfg.c (replace_ssa_name): Revert obsolete changes.
diff --git gcc/config/nvptx/nvptx.c gcc/config/nvptx/nvptx.c
index e81cbba..fcebf02 100644
--- gcc/config/nvptx/nvptx.c
+++ gcc/config/nvptx/nvptx.c
@@ -4322,15 +4322,6 @@  nvptx_xform_fork_join (gcall *call, const int dims[],
   return false;
 }
 
-/* Check lock & unlock.  We don't reqyire any locks.  */
-
-static bool
-nvptx_xform_lock (gcall *ARG_UNUSED (call),
-		  const int *ARG_UNUSED (dims), unsigned ARG_UNUSED (ifn_code))
-{
-  return true;
-}
-
 static tree
 nvptx_get_worker_red_addr (tree type, tree rid, tree lid)
 {