Re: [Fortran, Patch] Coarray: libcaf patch for _gfortran_caf_deregister

[Tobias Burnus <burnus at net-b dot de>]

On 08/29/2011 12:33 AM, Mikael Morin wrote:
> diff --git a/libgfortran/caf/mpi.c b/libgfortran/caf/mpi.c
> index ea4c0f0..711c6ee 100644
> --- a/libgfortran/caf/mpi.c
> +++ b/libgfortran/caf/mpi.c
> @@ -103,7 +103,7 @@ _gfortran_caf_finalize (void)
>   {
>     while (caf_static_list != NULL)
>       {
> -      free(caf_static_list->token[caf_this_image-1]);
> +      free (caf_static_list->token[caf_this_image-1]);
>         caf_static_list = caf_static_list->prev;
>       }
>
> Not something introduced by this patch, but I would like to point that
> caf_static_list should be freed too.

And also "caf_static_list->token".

> +  if (stat)
> +    *stat = 0;
> +
> +  free (token[caf_this_image-1]);
>   }
>
>
> To be consistent with _gfortran_caf_register, I think you should be freeing
> token itself (the whole array).
> OK with that change.

I did so - also for single.c. While checking the result, I also found a 
bug in the registering code for static coarrays, where the wrong (later 
introduced) enum was used.

Thanks for the thorough review! Attached patch was committed as Rev. 178193.

Tobias
gcc/fortran/
2011-08-29  Tobias Burnus  <burnus@net-b.de>

	* trans-decl.c (generate_coarray_sym_init): Use
	GFC_CAF_COARRAY_STATIC for static coarrays.

libgfortan/
2011-08-29  Tobias Burnus  <burnus@net-b.de>

	* caf/libcaf.h (_gfortran_caf_deregister): Update prototype.
	* caf/mpi.c (_gfortran_caf_deregister): Modify prototype,
	actually free memory and add error diagnostic.
	(_gfortran_caf_finalize): Add additional free calls.
	* caf/single.c (_gfortran_caf_deregister): Modify prototype,
	actually free memory and add error diagnostic.
	(_gfortran_caf_finalize): Add additional free calls.

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index c85e20c..ead8acf 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -4241,7 +4241,7 @@ generate_coarray_sym_init (gfc_symbol *sym)
 
   tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_register, 6, size,
 			     build_int_cst (integer_type_node,
-					    GFC_CAF_COARRAY_ALLOC), /* type.  */
+					    GFC_CAF_COARRAY_STATIC), /* type.  */
 			     token, null_pointer_node, /* token, stat.  */
 			     null_pointer_node, /* errgmsg, errmsg_len.  */
 			     build_int_cst (integer_type_node, 0));
diff --git a/libgfortran/caf/libcaf.h b/libgfortran/caf/libcaf.h
index 4fe09e4..e6be7ce 100644
--- a/libgfortran/caf/libcaf.h
+++ b/libgfortran/caf/libcaf.h
@@ -69,7 +69,7 @@ void _gfortran_caf_finalize (void);
 
 void * _gfortran_caf_register (ptrdiff_t, caf_register_t, void **, int *,
 			       char *, int);
-int _gfortran_caf_deregister (void **);
+void _gfortran_caf_deregister (void **, int *, char *, int);
 
 
 void _gfortran_caf_sync_all (int *, char *, int);
diff --git a/libgfortran/caf/mpi.c b/libgfortran/caf/mpi.c
index ea4c0f0..c69c5b9 100644
--- a/libgfortran/caf/mpi.c
+++ b/libgfortran/caf/mpi.c
@@ -103,8 +103,12 @@ _gfortran_caf_finalize (void)
 {
   while (caf_static_list != NULL)
     {
-      free(caf_static_list->token[caf_this_image-1]);
-      caf_static_list = caf_static_list->prev;
+      caf_static_t *tmp = caf_static_list->prev;
+
+      free (caf_static_list->token[caf_this_image-1]);
+      free (caf_static_list->token);
+      free (caf_static_list);
+      caf_static_list = tmp;
     }
 
   if (!caf_mpi_initialized)
@@ -187,10 +191,37 @@ error:
 }
 
 
-int
-_gfortran_caf_deregister (void **token __attribute__ ((unused)))
+void
+_gfortran_caf_deregister (void **token, int *stat, char *errmsg, int errmsg_len)
 {
-  return 0;
+  if (unlikely (caf_is_finalized))
+    {
+      const char msg[] = "Failed to deallocate coarray - "
+			  "there are stopped images";
+      if (stat)
+	{
+	  *stat = STAT_STOPPED_IMAGE;
+	
+	  if (errmsg_len > 0)
+	    {
+	      int len = ((int) sizeof (msg) - 1 > errmsg_len)
+			? errmsg_len : (int) sizeof (msg) - 1;
+	      memcpy (errmsg, msg, len);
+	      if (errmsg_len > len)
+		memset (&errmsg[len], ' ', errmsg_len-len);
+	    }
+	  return;
+	}
+      caf_runtime_error (msg);
+    }
+
+  _gfortran_caf_sync_all (NULL, NULL, 0);
+
+  if (stat)
+    *stat = 0;
+
+  free (token[caf_this_image-1]);
+  free (token);
 }
 
 
@@ -267,7 +298,7 @@ _gfortran_caf_sync_images (int count, int images[], int *stat, char *errmsg,
     }
 
   /* Handle SYNC IMAGES(*).  */
-  if (unlikely(caf_is_finalized))
+  if (unlikely (caf_is_finalized))
     ierr = STAT_STOPPED_IMAGE;
   else
     ierr = MPI_Barrier (MPI_COMM_WORLD);
diff --git a/libgfortran/caf/single.c b/libgfortran/caf/single.c
index 09cc62f..5353c7b 100644
--- a/libgfortran/caf/single.c
+++ b/libgfortran/caf/single.c
@@ -71,8 +71,11 @@ _gfortran_caf_finalize (void)
 {
   while (caf_static_list != NULL)
     {
-      free(caf_static_list->token[0]);
-      caf_static_list = caf_static_list->prev;
+      caf_static_t *tmp = caf_static_list->prev;
+      free (caf_static_list->token[0]);
+      free (caf_static_list->token);
+      free (caf_static_list);
+      caf_static_list = tmp;
     }
 }
 
@@ -121,10 +124,16 @@ _gfortran_caf_register (ptrdiff_t size, caf_register_t type, void **token,
 }
 
 
-int
-_gfortran_caf_deregister (void **token __attribute__ ((unused)))
+void
+_gfortran_caf_deregister (void **token, int *stat,
+			  char *errmsg __attribute__ ((unused)),
+			  int errmsg_len __attribute__ ((unused)))
 {
-  return 0;
+  free (*token);
+  free (token);
+
+  if (stat)
+    *stat = 0;
 }