public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [gomp4] remove kernel-specific launch
@ 2015-07-19 16:14 Nathan Sidwell
  2015-07-19 17:13 ` Tom de Vries
  2015-07-19 21:05 ` Thomas Schwinge
  0 siblings, 2 replies; 9+ messages in thread
From: Nathan Sidwell @ 2015-07-19 16:14 UTC (permalink / raw)
  To: Tom de Vries; +Cc: GCC Patches

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

Tom,
as we discussed this patch removes the GOACC_kernels call, which forwards to 
GOACC_parallel.  We simply call GOACC_parallel directly.

ok for gomp4 branch?

nathan

[-- Attachment #2: gomp4-kernel.patch --]
[-- Type: text/x-patch, Size: 4146 bytes --]

2015-07-19  Nathan Sidwell  <nathan@codesourcery.com>

	gcc/
	* omp-low.c (expand_omp_target): Convert to
	BUILT_IN_GOACC_PARALLEL. Remove BUILT_IN_GOACC_KERNELS cases.
	* omp-builtins.def (BUILT_IN_GOACC_KERNELS): Delete.

	libgomp/
	* oacc-parallel.c (GOACC_kernels): Delete.

Index: gcc/omp-builtins.def
===================================================================
--- gcc/omp-builtins.def	(revision 225982)
+++ gcc/omp-builtins.def	(working copy)
@@ -48,9 +48,6 @@ DEF_GOACC_BUILTIN_FNSPEC (BUILT_IN_GOACC
 			  BT_FN_VOID_INT_OMPFN_SIZE_PTR_PTR_PTR_INT_INT_INT_SIZE_INT_INT_VAR,
 			  ATTR_FNSPEC_DOT_DOT_DOT_DOT_r_r_r_NOTHROW_LIST,
 			  ATTR_NOTHROW_LIST, "....rrr")
-DEF_GOACC_BUILTIN (BUILT_IN_GOACC_KERNELS, "GOACC_kernels",
-		   BT_FN_VOID_INT_OMPFN_SIZE_PTR_PTR_PTR_INT_INT_INT_SIZE_INT_INT_VAR,
-		   ATTR_NOTHROW_LIST)
 DEF_GOACC_BUILTIN (BUILT_IN_GOACC_PARALLEL, "GOACC_parallel",
 		   BT_FN_VOID_INT_OMPFN_SIZE_PTR_PTR_PTR_INT_INT_INT_SIZE_INT_INT_VAR,
 		   ATTR_NOTHROW_LIST)
Index: gcc/omp-low.c
===================================================================
--- gcc/omp-low.c	(revision 225982)
+++ gcc/omp-low.c	(working copy)
@@ -9339,14 +9339,15 @@ expand_omp_target (struct omp_region *re
 	  /* Don't emit the library call.  We've already done that.  */
 	  do_emit_library_call = false;
 	  /* Transform BUILT_IN_GOACC_KERNELS_INTERNAL into
-	     BUILT_IN_GOACC_KERNELS_INTERNAL.  Now that the function body will be
-	     split off, we can no longer regard the omp_data_array reference as
-	     non-escaping.  */
+	     BUILT_IN_GOACC_PARALLELL.  Now that the function
+	     body will be split off, we can no longer regard the
+	     omp_data_array reference as non-escaping.  */
 	  gsi = gsi_last_bb (entry_bb);
 	  gsi_prev (&gsi);
 	  gcall *call = as_a <gcall *> (gsi_stmt (gsi));
-	  gcc_assert (gimple_call_builtin_p (call, BUILT_IN_GOACC_KERNELS_INTERNAL));
-	  tree fndecl = builtin_decl_explicit (BUILT_IN_GOACC_KERNELS);
+	  gcc_assert (gimple_call_builtin_p
+		      (call, BUILT_IN_GOACC_KERNELS_INTERNAL));
+	  tree fndecl = builtin_decl_explicit (BUILT_IN_GOACC_PARALLEL);
 	  gimple_call_set_fndecl (call, fndecl);
 	  gimple_call_set_fntype (call, TREE_TYPE (fndecl));
 	  gimple_call_reset_alias_info (call);
@@ -9723,7 +9724,6 @@ expand_omp_target (struct omp_region *re
     case BUILT_IN_GOACC_DATA_START:
     case BUILT_IN_GOACC_DECLARE:
     case BUILT_IN_GOACC_ENTER_EXIT_DATA:
-    case BUILT_IN_GOACC_KERNELS:
     case BUILT_IN_GOACC_KERNELS_INTERNAL:
     case BUILT_IN_GOACC_PARALLEL:
     case BUILT_IN_GOACC_UPDATE:
@@ -9743,7 +9743,6 @@ expand_omp_target (struct omp_region *re
     case BUILT_IN_GOMP_TARGET_DATA:
     case BUILT_IN_GOMP_TARGET_UPDATE:
       break;
-    case BUILT_IN_GOACC_KERNELS:
     case BUILT_IN_GOACC_KERNELS_INTERNAL:
     case BUILT_IN_GOACC_PARALLEL:
       {
Index: libgomp/oacc-parallel.c
===================================================================
--- libgomp/oacc-parallel.c	(revision 225982)
+++ libgomp/oacc-parallel.c	(working copy)
@@ -530,37 +530,6 @@ GOACC_enter_exit_data (int device, size_
   acc_dev->openacc.async_set_async_func (acc_async_sync);
 }
 
-void
-GOACC_kernels (int device, void (*fn) (void *),
-	       size_t mapnum, void **hostaddrs, size_t *sizes,
-	       unsigned short *kinds,
-	       int num_gangs, int num_workers, int vector_length,
-	       size_t shared_size, int async, int num_waits, ...)
-{
-#ifdef HAVE_INTTYPES_H
-  gomp_debug (0, "%s: mapnum=%"PRIu64", hostaddrs=%p, sizes=%p, kinds=%p\n",
-	      __FUNCTION__, (uint64_t) mapnum, hostaddrs, sizes, kinds);
-#else
-  gomp_debug (0, "%s: mapnum=%lu, hostaddrs=%p, sizes=%p, kinds=%p\n",
-	      __FUNCTION__, (unsigned long) mapnum, hostaddrs, sizes, kinds);
-#endif
-
-  va_list ap;
-
-  goacc_lazy_initialize ();
-
-  va_start (ap, num_waits);
-
-  if (num_waits > 0)
-    goacc_wait (async, num_waits, ap);
-
-  va_end (ap);
-
-  GOACC_parallel (device, fn, mapnum, hostaddrs, sizes, kinds,
-		  num_gangs, num_workers, vector_length, shared_size,
-		  async, 0);
-}
-
 static void
 goacc_wait (int async, int num_waits, va_list ap)
 {

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gomp4] remove kernel-specific launch
  2015-07-19 16:14 [gomp4] remove kernel-specific launch Nathan Sidwell
@ 2015-07-19 17:13 ` Tom de Vries
  2015-07-19 21:05 ` Thomas Schwinge
  1 sibling, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2015-07-19 17:13 UTC (permalink / raw)
  To: Nathan Sidwell; +Cc: GCC Patches

On 19/07/15 14:27, Nathan Sidwell wrote:
> Tom,
> as we discussed this patch removes the GOACC_kernels call, which
> forwards to GOACC_parallel.  We simply call GOACC_parallel directly.
>
> ok for gomp4 branch?
>

Hi Nathan,

yes, that looks good.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gomp4] remove kernel-specific launch
  2015-07-19 16:14 [gomp4] remove kernel-specific launch Nathan Sidwell
  2015-07-19 17:13 ` Tom de Vries
@ 2015-07-19 21:05 ` Thomas Schwinge
  2015-07-19 21:08   ` Tom de Vries
  2015-07-19 21:10   ` Nathan Sidwell
  1 sibling, 2 replies; 9+ messages in thread
From: Thomas Schwinge @ 2015-07-19 21:05 UTC (permalink / raw)
  To: Nathan Sidwell, Tom de Vries; +Cc: GCC Patches

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

Hi!

On Sun, 19 Jul 2015 08:27:44 -0400, Nathan Sidwell <nathan@acm.org> wrote:
> Tom,
> as we discussed this patch removes the GOACC_kernels call, which forwards to 
> GOACC_parallel.  We simply call GOACC_parallel directly.
> 
> ok for gomp4 branch?

> 	gcc/
> 	* omp-low.c (expand_omp_target): Convert to
> 	BUILT_IN_GOACC_PARALLEL. Remove BUILT_IN_GOACC_KERNELS cases.
> 	* omp-builtins.def (BUILT_IN_GOACC_KERNELS): Delete.
> 
> 	libgomp/
> 	* oacc-parallel.c (GOACC_kernels): Delete.

Yes, thanks for the cleanup.  Please also adjust/remove the following:

    gcc/tree-parloops.c:    /* Remove GOACC_kernels.  */
    libgomp/libgomp.map:    GOACC_kernels;
    libgomp/libgomp_g.h:extern void GOACC_kernels (int, void (*) (void *), size_t,

Does it make sense then to rename GOACC_kernels_internal to
GOACC_kernels?


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gomp4] remove kernel-specific launch
  2015-07-19 21:05 ` Thomas Schwinge
@ 2015-07-19 21:08   ` Tom de Vries
  2015-07-19 21:10   ` Nathan Sidwell
  1 sibling, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2015-07-19 21:08 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: Nathan Sidwell, GCC Patches

On 19/07/15 22:30, Thomas Schwinge wrote:
> Does it make sense then to rename GOACC_kernels_internal to
> GOACC_kernels?

I prefer to keep the name GOACC_kernels_internal, IMO it's clearer.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gomp4] remove kernel-specific launch
  2015-07-19 21:05 ` Thomas Schwinge
  2015-07-19 21:08   ` Tom de Vries
@ 2015-07-19 21:10   ` Nathan Sidwell
  2015-07-20 12:08     ` Tom de Vries
  1 sibling, 1 reply; 9+ messages in thread
From: Nathan Sidwell @ 2015-07-19 21:10 UTC (permalink / raw)
  To: Thomas Schwinge, Nathan Sidwell, Tom de Vries; +Cc: GCC Patches

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

On 07/19/15 16:30, Thomas Schwinge wrote:

>      gcc/tree-parloops.c:    /* Remove GOACC_kernels.  */
>      libgomp/libgomp.map:    GOACC_kernels;
>      libgomp/libgomp_g.h:extern void GOACC_kernels (int, void (*) (void *), size_t,

I fixed all byt the parloops comment.  That comment didn't really make sense to 
me -- it seems to be doing something with the pragma not the call.   Perhaps Tom 
could correct/clarify it?

> Does it make sense then to rename GOACC_kernels_internal to
> GOACC_kernels?

I  agree with Tom.  But perhaps it should be an internal fn? IIUC those are for 
pseudo-funcs that should be converted to something else before the end of 
compilation.

nathan

[-- Attachment #2: gomp4-kernel-2.patch --]
[-- Type: text/x-patch, Size: 5223 bytes --]

2015-07-19  Nathan Sidwell  <nathan@codesourcery.com>

	gcc/
	* omp-low.c (expand_omp_target): Convert to
	BUILT_IN_GOACC_PARALLEL. Remove BUILT_IN_GOACC_KERNELS cases.
	* omp-builtins.def (BUILT_IN_GOACC_KERNELS): Delete.

	libgomp/
	* oacc-parallel.c (GOACC_kernels): Delete.
	* libgomp.map: Remove GOACC_kernels.
	* libgomp_g.h (GOACC_kernels): Remove.

Index: gcc/omp-low.c
===================================================================
--- gcc/omp-low.c	(revision 225982)
+++ gcc/omp-low.c	(working copy)
@@ -9339,14 +9339,15 @@ expand_omp_target (struct omp_region *re
 	  /* Don't emit the library call.  We've already done that.  */
 	  do_emit_library_call = false;
 	  /* Transform BUILT_IN_GOACC_KERNELS_INTERNAL into
-	     BUILT_IN_GOACC_KERNELS_INTERNAL.  Now that the function body will be
-	     split off, we can no longer regard the omp_data_array reference as
-	     non-escaping.  */
+	     BUILT_IN_GOACC_PARALLELL.  Now that the function
+	     body will be split off, we can no longer regard the
+	     omp_data_array reference as non-escaping.  */
 	  gsi = gsi_last_bb (entry_bb);
 	  gsi_prev (&gsi);
 	  gcall *call = as_a <gcall *> (gsi_stmt (gsi));
-	  gcc_assert (gimple_call_builtin_p (call, BUILT_IN_GOACC_KERNELS_INTERNAL));
-	  tree fndecl = builtin_decl_explicit (BUILT_IN_GOACC_KERNELS);
+	  gcc_assert (gimple_call_builtin_p
+		      (call, BUILT_IN_GOACC_KERNELS_INTERNAL));
+	  tree fndecl = builtin_decl_explicit (BUILT_IN_GOACC_PARALLEL);
 	  gimple_call_set_fndecl (call, fndecl);
 	  gimple_call_set_fntype (call, TREE_TYPE (fndecl));
 	  gimple_call_reset_alias_info (call);
@@ -9723,7 +9724,6 @@ expand_omp_target (struct omp_region *re
     case BUILT_IN_GOACC_DATA_START:
     case BUILT_IN_GOACC_DECLARE:
     case BUILT_IN_GOACC_ENTER_EXIT_DATA:
-    case BUILT_IN_GOACC_KERNELS:
     case BUILT_IN_GOACC_KERNELS_INTERNAL:
     case BUILT_IN_GOACC_PARALLEL:
     case BUILT_IN_GOACC_UPDATE:
@@ -9743,7 +9743,6 @@ expand_omp_target (struct omp_region *re
     case BUILT_IN_GOMP_TARGET_DATA:
     case BUILT_IN_GOMP_TARGET_UPDATE:
       break;
-    case BUILT_IN_GOACC_KERNELS:
     case BUILT_IN_GOACC_KERNELS_INTERNAL:
     case BUILT_IN_GOACC_PARALLEL:
       {
Index: gcc/omp-builtins.def
===================================================================
--- gcc/omp-builtins.def	(revision 225982)
+++ gcc/omp-builtins.def	(working copy)
@@ -48,9 +48,6 @@ DEF_GOACC_BUILTIN_FNSPEC (BUILT_IN_GOACC
 			  BT_FN_VOID_INT_OMPFN_SIZE_PTR_PTR_PTR_INT_INT_INT_SIZE_INT_INT_VAR,
 			  ATTR_FNSPEC_DOT_DOT_DOT_DOT_r_r_r_NOTHROW_LIST,
 			  ATTR_NOTHROW_LIST, "....rrr")
-DEF_GOACC_BUILTIN (BUILT_IN_GOACC_KERNELS, "GOACC_kernels",
-		   BT_FN_VOID_INT_OMPFN_SIZE_PTR_PTR_PTR_INT_INT_INT_SIZE_INT_INT_VAR,
-		   ATTR_NOTHROW_LIST)
 DEF_GOACC_BUILTIN (BUILT_IN_GOACC_PARALLEL, "GOACC_parallel",
 		   BT_FN_VOID_INT_OMPFN_SIZE_PTR_PTR_PTR_INT_INT_INT_SIZE_INT_INT_VAR,
 		   ATTR_NOTHROW_LIST)
Index: libgomp/libgomp.map
===================================================================
--- libgomp/libgomp.map	(revision 225982)
+++ libgomp/libgomp.map	(working copy)
@@ -331,7 +331,6 @@ GOACC_2.0.GOMP_4_BRANCH {
   global:
 	GOACC_deviceptr;
 	GOACC_get_ganglocal_ptr;
-	GOACC_kernels;
 	GOACC_register_static;
 } GOACC_2.0;
 
Index: libgomp/oacc-parallel.c
===================================================================
--- libgomp/oacc-parallel.c	(revision 225982)
+++ libgomp/oacc-parallel.c	(working copy)
@@ -530,37 +530,6 @@ GOACC_enter_exit_data (int device, size_
   acc_dev->openacc.async_set_async_func (acc_async_sync);
 }
 
-void
-GOACC_kernels (int device, void (*fn) (void *),
-	       size_t mapnum, void **hostaddrs, size_t *sizes,
-	       unsigned short *kinds,
-	       int num_gangs, int num_workers, int vector_length,
-	       size_t shared_size, int async, int num_waits, ...)
-{
-#ifdef HAVE_INTTYPES_H
-  gomp_debug (0, "%s: mapnum=%"PRIu64", hostaddrs=%p, sizes=%p, kinds=%p\n",
-	      __FUNCTION__, (uint64_t) mapnum, hostaddrs, sizes, kinds);
-#else
-  gomp_debug (0, "%s: mapnum=%lu, hostaddrs=%p, sizes=%p, kinds=%p\n",
-	      __FUNCTION__, (unsigned long) mapnum, hostaddrs, sizes, kinds);
-#endif
-
-  va_list ap;
-
-  goacc_lazy_initialize ();
-
-  va_start (ap, num_waits);
-
-  if (num_waits > 0)
-    goacc_wait (async, num_waits, ap);
-
-  va_end (ap);
-
-  GOACC_parallel (device, fn, mapnum, hostaddrs, sizes, kinds,
-		  num_gangs, num_workers, vector_length, shared_size,
-		  async, 0);
-}
-
 static void
 goacc_wait (int async, int num_waits, va_list ap)
 {
Index: libgomp_g.h
===================================================================
--- libgomp_g.h	(revision 225982)
+++ libgomp_g.h	(working copy)
@@ -222,9 +222,6 @@ extern void GOACC_data_start (int, size_
 extern void GOACC_data_end (void);
 extern void GOACC_enter_exit_data (int, size_t, void **,
 				   size_t *, unsigned short *, int, int, ...);
-extern void GOACC_kernels (int, void (*) (void *), size_t,
-			   void **, size_t *, unsigned short *, int, int, int,
-			   size_t, int, int, ...);
 extern void GOACC_parallel (int, void (*) (void *), size_t,
 			    void **, size_t *, unsigned short *, int, int, int,
 			    size_t, int, int, ...);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gomp4] remove kernel-specific launch
  2015-07-19 21:10   ` Nathan Sidwell
@ 2015-07-20 12:08     ` Tom de Vries
  2015-07-20 12:45       ` Nathan Sidwell
  2015-07-21 10:23       ` Thomas Schwinge
  0 siblings, 2 replies; 9+ messages in thread
From: Tom de Vries @ 2015-07-20 12:08 UTC (permalink / raw)
  To: Nathan Sidwell, Thomas Schwinge, Nathan Sidwell, Tom de Vries; +Cc: GCC Patches

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

On 19/07/15 23:08, Nathan Sidwell wrote:
> On 07/19/15 16:30, Thomas Schwinge wrote:
>
>>      gcc/tree-parloops.c:    /* Remove GOACC_kernels.  */
>>      libgomp/libgomp.map:    GOACC_kernels;
>>      libgomp/libgomp_g.h:extern void GOACC_kernels (int, void (*)
>> (void *), size_t,
>
> I fixed all byt the parloops comment.  That comment didn't really make
> sense to me -- it seems to be doing something with the pragma not the
> call.   Perhaps Tom could correct/clarify it?
>

Committed as attached.

>> Does it make sense then to rename GOACC_kernels_internal to
>> GOACC_kernels?
>
> I  agree with Tom.  But perhaps it should be an internal fn? IIUC those
> are for pseudo-funcs that should be converted to something else before
> the end of compilation.

Turning it into an internal fn will make it harder to convert a 
GOACC_kernels_internal call into a GOACC_parallel call, which we're 
doing here in omp-low.c:
...
           tree fndecl = builtin_decl_explicit (BUILT_IN_GOACC_PARALLEL);
           gimple_call_set_fndecl (call, fndecl);
           gimple_call_set_fntype (call, TREE_TYPE (fndecl));
           gimple_call_reset_alias_info (call);
  ...

Thanks,
- Tom


[-- Attachment #2: 0001-Update-create_parallel_loop-for-remove-GOACC_kernels.patch --]
[-- Type: text/x-patch, Size: 2046 bytes --]

Update create_parallel_loop for remove GOACC_kernels

2015-07-20  Tom de Vries  <tom@codesourcery.com>

	* tree-parloops.c (create_parallel_loop): Update comments for removal of
	GOACC_kernels.  Rename goacc_kernels variable into
	goacc_kernels_internal.
---
 gcc/tree-parloops.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c
index 149c336..7111f93 100644
--- a/gcc/tree-parloops.c
+++ b/gcc/tree-parloops.c
@@ -2045,11 +2045,12 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
     }
   else
     {
-      /* Create oacc parallel pragma based on oacc kernels pragma.  */
+      /* Create oacc parallel pragma based on oacc kernels pragma and
+	 GOAC_kernels_internal call.  */
       gomp_target *kernels = as_a <gomp_target *> (gsi_stmt (gsi));
 
       gsi_prev (&gsi);
-      gcall *goacc_kernels = as_a <gcall *> (gsi_stmt (gsi));
+      gcall *goacc_kernels_internal = as_a <gcall *> (gsi_stmt (gsi));
 
       tree clauses = gimple_omp_target_clauses (kernels);
       /* FIXME: We need a more intelligent mapping onto vector, gangs,
@@ -2070,7 +2071,8 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
       gimple_omp_target_set_child_fn (stmt, child_fn);
       tree data_arg = gimple_omp_target_data_arg (kernels);
       gimple_omp_target_set_data_arg (stmt, data_arg);
-      tree ganglocal_size = gimple_call_arg (goacc_kernels, /* TODO */ 9);
+      tree ganglocal_size
+	= gimple_call_arg (goacc_kernels_internal, /* TODO */ 9);
       gimple_omp_target_set_ganglocal_size (stmt, ganglocal_size);
 
       gimple_set_location (stmt, loc);
@@ -2085,7 +2087,7 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
 	/* Insert pragma acc parallel.  */
 	gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
 
-	/* Remove GOACC_kernels.  */
+	/* Remove GOACC_kernels_internal call.  */
 	replace_uses_by (gimple_vdef (gsi_stmt (gsi2)),
 			 gimple_vuse (gsi_stmt (gsi2)));
 	gsi_remove (&gsi2, true);
-- 
1.9.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gomp4] remove kernel-specific launch
  2015-07-20 12:08     ` Tom de Vries
@ 2015-07-20 12:45       ` Nathan Sidwell
  2015-07-21 10:23       ` Thomas Schwinge
  1 sibling, 0 replies; 9+ messages in thread
From: Nathan Sidwell @ 2015-07-20 12:45 UTC (permalink / raw)
  To: Tom de Vries, Thomas Schwinge, Nathan Sidwell, Tom de Vries; +Cc: GCC Patches

On 07/20/15 05:54, Tom de Vries wrote:

> Turning it into an internal fn will make it harder to convert a
> GOACC_kernels_internal call into a GOACC_parallel call, which we're doing here

I wondered ...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gomp4] remove kernel-specific launch
  2015-07-20 12:08     ` Tom de Vries
  2015-07-20 12:45       ` Nathan Sidwell
@ 2015-07-21 10:23       ` Thomas Schwinge
  2015-07-21 12:07         ` Tom de Vries
  1 sibling, 1 reply; 9+ messages in thread
From: Thomas Schwinge @ 2015-07-21 10:23 UTC (permalink / raw)
  To: Nathan Sidwell, Tom de Vries; +Cc: GCC Patches

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

Hi!

Thanks for the cleanup!

On Mon, 20 Jul 2015 11:54:48 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 19/07/15 23:08, Nathan Sidwell wrote:
> > On 07/19/15 16:30, Thomas Schwinge wrote:
> >> Does it make sense then to rename GOACC_kernels_internal to
> >> GOACC_kernels?

> > [...] perhaps it should be an internal fn? IIUC those
> > are for pseudo-funcs that should be converted to something else before
> > the end of compilation.
> 
> Turning it into an internal fn will make it harder to convert a 
> GOACC_kernels_internal call into a GOACC_parallel call, which we're 
> doing here in omp-low.c:
> ...
>            tree fndecl = builtin_decl_explicit (BUILT_IN_GOACC_PARALLEL);
>            gimple_call_set_fndecl (call, fndecl);
>            gimple_call_set_fntype (call, TREE_TYPE (fndecl));
>            gimple_call_reset_alias_info (call);
>   ...

..., and also a similar transformation in gcc/tree-parloops.c, I think,
which I've not been too fond of ;-) anyway, because of:

> --- a/gcc/tree-parloops.c
> +++ b/gcc/tree-parloops.c
> @@ -2045,11 +2045,12 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
>      }
>    else
>      {
> -      /* Create oacc parallel pragma based on oacc kernels pragma.  */
> +      /* Create oacc parallel pragma based on oacc kernels pragma and
> +	 GOAC_kernels_internal call.  */
>        gomp_target *kernels = as_a <gomp_target *> (gsi_stmt (gsi));
>  
>        gsi_prev (&gsi);
> -      gcall *goacc_kernels = as_a <gcall *> (gsi_stmt (gsi));
> +      gcall *goacc_kernels_internal = as_a <gcall *> (gsi_stmt (gsi));
>  
>        tree clauses = gimple_omp_target_clauses (kernels);
>        /* FIXME: We need a more intelligent mapping onto vector, gangs,
> @@ -2070,7 +2071,8 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
>        gimple_omp_target_set_child_fn (stmt, child_fn);
>        tree data_arg = gimple_omp_target_data_arg (kernels);
>        gimple_omp_target_set_data_arg (stmt, data_arg);
> -      tree ganglocal_size = gimple_call_arg (goacc_kernels, /* TODO */ 9);
> +      tree ganglocal_size
> +	= gimple_call_arg (goacc_kernels_internal, /* TODO */ 9);
>        gimple_omp_target_set_ganglocal_size (stmt, ganglocal_size);

... this "clumsy" argument copying.

As I understand it, there is an implicit/non-obvious requirement that the
GOACC_kernels_internal and GOACC_parallel function/built-in signatures
match?  Does it thus make sense to add a comment to the definition of the
GOACC_kernels_internal, in gcc/omp-builtins.def, to describe that?


Committed in r226035:

commit fbf4d5131957d06b12ce19a1b870ec6b383acbfd
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Jul 21 10:18:07 2015 +0000

    More GOACC_kernels removal fixes
    
    ... for r225994, and r225989.
    
    	gcc/
    	* tree-parloops.c (create_parallel_loop): Typo fix.
    	libgomp/
    	* libgomp_g.h (GOACC_kernels): Remove.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@226035 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog.gomp     |    4 ++++
 gcc/tree-parloops.c    |    2 +-
 libgomp/ChangeLog.gomp |    5 ++++-
 libgomp/libgomp_g.h    |    3 ---
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git gcc/ChangeLog.gomp gcc/ChangeLog.gomp
index 7bb7c09..2351d85 100644
--- gcc/ChangeLog.gomp
+++ gcc/ChangeLog.gomp
@@ -1,3 +1,7 @@
+2015-07-21  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* tree-parloops.c (create_parallel_loop): Typo fix.
+
 2015-07-20  Nathan Sidwell  <nathan@codesourcery.com>
 
 	* config/nvptx/nvptx.c (nvptx_builtins): Delete enum.
diff --git gcc/tree-parloops.c gcc/tree-parloops.c
index 7111f93..a825e7b 100644
--- gcc/tree-parloops.c
+++ gcc/tree-parloops.c
@@ -2046,7 +2046,7 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
   else
     {
       /* Create oacc parallel pragma based on oacc kernels pragma and
-	 GOAC_kernels_internal call.  */
+	 GOACC_kernels_internal call.  */
       gomp_target *kernels = as_a <gomp_target *> (gsi_stmt (gsi));
 
       gsi_prev (&gsi);
diff --git libgomp/ChangeLog.gomp libgomp/ChangeLog.gomp
index 02a84dc..161255b 100644
--- libgomp/ChangeLog.gomp
+++ libgomp/ChangeLog.gomp
@@ -1,3 +1,7 @@
+2015-07-21  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* libgomp_g.h (GOACC_kernels): Remove.
+
 2015-07-20  Nathan Sidwell  <nathan@codesourcery.com>
 
 	* oacc-parallel.c (GOACC_parallel): Move variadic handling into
@@ -21,7 +25,6 @@
 
 	* oacc-parallel.c (GOACC_kernels): Delete.
 	* libgomp.map: Remove GOACC_kernels.
-	* libgomp_g.h (GOACC_kernels): Remove.
 
 2015-07-17  Nathan Sidwell  <nathan@codesourcery.com>
 
diff --git libgomp/libgomp_g.h libgomp/libgomp_g.h
index 943ee67..63ae972 100644
--- libgomp/libgomp_g.h
+++ libgomp/libgomp_g.h
@@ -222,9 +222,6 @@ extern void GOACC_data_start (int, size_t, void **, size_t *,
 extern void GOACC_data_end (void);
 extern void GOACC_enter_exit_data (int, size_t, void **,
 				   size_t *, unsigned short *, int, int, ...);
-extern void GOACC_kernels (int, void (*) (void *), size_t,
-			   void **, size_t *, unsigned short *, int, int, int,
-			   size_t, int, int, ...);
 extern void GOACC_parallel (int, void (*) (void *), size_t,
 			    void **, size_t *, unsigned short *, int, int, int,
 			    size_t, int, int, ...);


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gomp4] remove kernel-specific launch
  2015-07-21 10:23       ` Thomas Schwinge
@ 2015-07-21 12:07         ` Tom de Vries
  0 siblings, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2015-07-21 12:07 UTC (permalink / raw)
  To: Thomas Schwinge, Nathan Sidwell; +Cc: GCC Patches

On 21/07/15 12:19, Thomas Schwinge wrote:
> Hi!
>
> Thanks for the cleanup!
>
> On Mon, 20 Jul 2015 11:54:48 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> On 19/07/15 23:08, Nathan Sidwell wrote:
>>> On 07/19/15 16:30, Thomas Schwinge wrote:
>>>> Does it make sense then to rename GOACC_kernels_internal to
>>>> GOACC_kernels?
>
>>> [...] perhaps it should be an internal fn? IIUC those
>>> are for pseudo-funcs that should be converted to something else before
>>> the end of compilation.
>>
>> Turning it into an internal fn will make it harder to convert a
>> GOACC_kernels_internal call into a GOACC_parallel call, which we're
>> doing here in omp-low.c:
>> ...
>>             tree fndecl = builtin_decl_explicit (BUILT_IN_GOACC_PARALLEL);
>>             gimple_call_set_fndecl (call, fndecl);
>>             gimple_call_set_fntype (call, TREE_TYPE (fndecl));
>>             gimple_call_reset_alias_info (call);
>>    ...
>
> ..., and also a similar transformation in gcc/tree-parloops.c, I think,

That's not a similar transformation. The GOACC_kernels_internal call is 
removed.

> which I've not been too fond of ;-) anyway, because of:
>
>> --- a/gcc/tree-parloops.c
>> +++ b/gcc/tree-parloops.c
>> @@ -2045,11 +2045,12 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
>>       }
>>     else
>>       {
>> -      /* Create oacc parallel pragma based on oacc kernels pragma.  */
>> +      /* Create oacc parallel pragma based on oacc kernels pragma and
>> +	 GOAC_kernels_internal call.  */
>>         gomp_target *kernels = as_a <gomp_target *> (gsi_stmt (gsi));
>>
>>         gsi_prev (&gsi);
>> -      gcall *goacc_kernels = as_a <gcall *> (gsi_stmt (gsi));
>> +      gcall *goacc_kernels_internal = as_a <gcall *> (gsi_stmt (gsi));
>>
>>         tree clauses = gimple_omp_target_clauses (kernels);
>>         /* FIXME: We need a more intelligent mapping onto vector, gangs,
>> @@ -2070,7 +2071,8 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
>>         gimple_omp_target_set_child_fn (stmt, child_fn);
>>         tree data_arg = gimple_omp_target_data_arg (kernels);
>>         gimple_omp_target_set_data_arg (stmt, data_arg);
>> -      tree ganglocal_size = gimple_call_arg (goacc_kernels, /* TODO */ 9);
>> +      tree ganglocal_size
>> +	= gimple_call_arg (goacc_kernels_internal, /* TODO */ 9);
>>         gimple_omp_target_set_ganglocal_size (stmt, ganglocal_size);
>
> ... this "clumsy" argument copying.
>
> As I understand it, there is an implicit/non-obvious requirement that the
> GOACC_kernels_internal and GOACC_parallel function/built-in signatures
> match?

It's not a hard requirement, but it's probably a good idea.

Thanks,
- Tom


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-07-21 11:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-19 16:14 [gomp4] remove kernel-specific launch Nathan Sidwell
2015-07-19 17:13 ` Tom de Vries
2015-07-19 21:05 ` Thomas Schwinge
2015-07-19 21:08   ` Tom de Vries
2015-07-19 21:10   ` Nathan Sidwell
2015-07-20 12:08     ` Tom de Vries
2015-07-20 12:45       ` Nathan Sidwell
2015-07-21 10:23       ` Thomas Schwinge
2015-07-21 12:07         ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).