* [Patch, Fortra, Coarray] Free allocated memory for static coarrays.
@ 2011-06-16 9:45 Daniel Carrera
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Carrera @ 2011-06-16 9:45 UTC (permalink / raw)
To: gfortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]
Hello,
This is my second patch for GFortran. This patch adds a linked list to
keep track of the allocated memory of all static coarrays and ensures
that at the end of the program the memory is freed.
Additionally, I use MPI_Allgather in mpi.c to ensure that all images
know the address of the stored memory in the other images. This will be
needed when the images start sending data to each other.
There is no test case attached because there is no way to automatically
test for allocated memory.
Here is my ChangeLog to go into libgfortran/ChangeLog:
2011-06-16 Daniel Carrera <dcarrera@gmail.com>
* caf/single.c (_gfortran_caf_register): Store the address
of all static coarrays in a linked list.
(_gfortran_caf_finalize): Free memory of staic coarrays.
* caf/mpi.c (_gfortran_caf_register): Store the address
of all static coarrays in a linked list. Initialize MPI
if necessary.
(_gfortran_caf_finalize): Free memory of staic coarrays.
(_gfortran_caf_init): Check if MPI is already initialized
before initializing again.
* caf/libcaf.h: Add a type to caf_register_t to distinguish
static coarrays and add the type caf_static_t to make the
linked list of static coarrays.
Cheers,
Daniel.
--
I'm not overweight, I'm undertall.
[-- Attachment #2: caf-free-memory.patch --]
[-- Type: text/x-patch, Size: 4696 bytes --]
Index: libgfortran/caf/single.c
===================================================================
--- libgfortran/caf/single.c (revision 174944)
+++ libgfortran/caf/single.c (working copy)
@@ -35,7 +35,10 @@
Note: For performance reasons -fcoarry=single should be used
rather than this library. */
+/* Global variables. */
+caf_static_t *caf_static_list = NULL;
+
void
_gfortran_caf_init (int *argc __attribute__ ((unused)),
char ***argv __attribute__ ((unused)),
@@ -49,16 +52,31 @@
void
_gfortran_caf_finalize (void)
{
+ while (caf_static_list != NULL)
+ {
+ free(caf_static_list->token[0]);
+ caf_static_list = caf_static_list->prev;
+ }
}
void *
-_gfortran_caf_register (ptrdiff_t size,
- caf_register_t type __attribute__ ((unused)),
+_gfortran_caf_register (ptrdiff_t size, caf_register_t type,
void **token)
{
- *token = NULL;
- return malloc (size);
+ void *local;
+
+ local = malloc (size);
+ token = malloc (sizeof (void*) * 1);
+
+ if (type == CAF_REGTYPE_COARRAY_STATIC)
+ {
+ caf_static_t *tmp = malloc (sizeof (caf_static_t));
+ tmp->prev = caf_static_list;
+ tmp->token = token;
+ caf_static_list = tmp;
+ }
+ return local;
}
Index: libgfortran/caf/libcaf.h
===================================================================
--- libgfortran/caf/libcaf.h (revision 174944)
+++ libgfortran/caf/libcaf.h (working copy)
@@ -38,15 +38,23 @@
#define STAT_LOCKED_OTHER_IMAGE 2
#define STAT_STOPPED_IMAGE 3
-
+/* Describes what type of array we are registerring. */
typedef enum caf_register_t {
- CAF_REGTYPE_COARRAY,
+ CAF_REGTYPE_COARRAY_STATIC,
+ CAF_REGTYPE_COARRAY_ALLOC,
CAF_REGTYPE_LOCK,
CAF_REGTYPE_LOCK_COMP
}
caf_register_t;
+/* Linked list of static coarrays registered. */
+typedef struct caf_static_t {
+ void **token;
+ struct caf_static_t *prev;
+}
+caf_static_t;
+
void _gfortran_caf_init (int *, char ***, int *, int *);
void _gfortran_caf_finalize (void);
Index: libgfortran/caf/mpi.c
===================================================================
--- libgfortran/caf/mpi.c (revision 174944)
+++ libgfortran/caf/mpi.c (working copy)
@@ -42,7 +42,10 @@
static int caf_this_image;
static int caf_num_images;
+caf_static_t *caf_static_list = NULL;
+
+
/* Initialize coarray program. This routine assumes that no other
MPI initialization happened before; otherwise MPI_Initialized
had to be used. As the MPI library might modify the command-line
@@ -52,36 +55,68 @@
void
_gfortran_caf_init (int *argc, char ***argv, int *this_image, int *num_images)
{
- /* caf_mpi_initialized is only true if the main program is not written in
- Fortran. */
- MPI_Initialized (&caf_mpi_initialized);
- if (!caf_mpi_initialized)
- MPI_Init (argc, argv);
+ if (caf_num_images == 0)
+ {
+ /* caf_mpi_initialized is only true if the main program is
+ not written in Fortran. */
+ MPI_Initialized (&caf_mpi_initialized);
+ if (!caf_mpi_initialized)
+ MPI_Init (argc, argv);
- MPI_Comm_rank (MPI_COMM_WORLD, &caf_this_image);
- *this_image = ++caf_this_image;
- MPI_Comm_size (MPI_COMM_WORLD, &caf_num_images);
- *num_images = caf_num_images;
+ MPI_Comm_size (MPI_COMM_WORLD, &caf_num_images);
+ MPI_Comm_rank (MPI_COMM_WORLD, &caf_this_image);
+ caf_this_image++;
+ }
+
+ if (this_image)
+ this_image = caf_this_image;
+ if (num_images)
+ num_images = caf_num_images;
}
+
/* Finalize coarray program. */
void
_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;
+ }
+
if (!caf_mpi_initialized)
MPI_Finalize ();
}
void *
-_gfortran_caf_register (ptrdiff_t size,
- caf_register_t type __attribute__ ((unused)),
+_gfortran_caf_register (ptrdiff_t size, caf_register_t type,
void **token)
{
- *token = NULL;
- return malloc (size);
+ void *local;
+
+ local = malloc (size);
+ token = malloc (sizeof (void*) * caf_num_images);
+
+ /* Start MPI if not already started. */
+ if (caf_num_images == 0)
+ _gfortran_caf_init (NULL, NULL, NULL, NULL);
+
+ /* token[img-1] is the address of the token in image "img". */
+ MPI_Allgather (local, sizeof (void*), MPI_BYTE,
+ token[0], sizeof (void*), MPI_BYTE, MPI_COMM_WORLD);
+
+ if (type == CAF_REGTYPE_COARRAY_STATIC)
+ {
+ caf_static_t *tmp = malloc (sizeof (caf_static_t));
+ tmp->prev = caf_static_list;
+ tmp->token = token;
+ caf_static_list = tmp;
+ }
+ return local;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, Fortra, Coarray] Free allocated memory for static coarrays.
@ 2011-06-16 12:55 Tobias Burnus
2011-06-16 15:17 ` Daniel Carrera
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2011-06-16 12:55 UTC (permalink / raw)
To: Daniel Carrera, gcc-patches, fortran
Hi Daniel,
Daniel Carrera wrote:
> This patch adds a linked list to keep track of the allocated
> memory of all static coarrays and ensures that at the end of
> the program the memory is freed.
+_gfortran_caf_register (ptrdiff_t size, caf_register_t type,
void **token)
Between "size," and "caf_register_t" is a <tab> instead of a simple " ".
_gfortran_caf_init (int *argc, char ***argv, int *this_image, int *num_images)
[...]
+ if (this_image)
+ this_image = caf_this_image;
+ if (num_images)
+ num_images = caf_num_images;
As you mentioned yourself (in a private mail, commenting on my code):
There is a "*" missing at the left side of the assignment lines.
+_gfortran_caf_register (ptrdiff_t size, caf_register_t type,
void **token)
[...]
+ token = malloc (sizeof (void*) * caf_num_images);
+
+ /* Start MPI if not already started. */
+ if (caf_num_images == 0)
+ _gfortran_caf_init (NULL, NULL, NULL, NULL);
Wrong order for the "token" allocation: You first need to know how many
images exists, before you can use "caf_num_images".
+ MPI_Allgather (local, sizeof (void*), MPI_BYTE,
There is a <tab> character between "local," and "sizeof"; it should
have been a space.
Have you tested the modifications with MPI? I mean: Compiling mpi.c,
and then in gcc/testsuite/gfortran.dg/coarray:
for I in *.f90; do echo ====== $I =====; \
mpif90 -fcoarray=lib $I mpi.o && mpiexec -np 3 ./a.out; done
Additionally, have you run test suite? It is sufficient to run recompile
gfortran (to create the latest libcaf_single.a from single.c) and then to
run
make check-gfortran RUNTESTFLAGS="caf.exp"
(caf.exp is sufficient as the rest of the testsuite does not use
libgfortran/caf/single.c, which is the only thing your patch modifies.)
Otherwise, your patch looks OK.
Tobias,
who has currently no access to the GCC source code and can hence not
test himself.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, Fortra, Coarray] Free allocated memory for static coarrays.
2011-06-16 12:55 Tobias Burnus
@ 2011-06-16 15:17 ` Daniel Carrera
2011-06-16 23:31 ` Tobias Burnus
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Carrera @ 2011-06-16 15:17 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc-patches, fortran
[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]
Hi Tobias,
New patch attached. In addition to the fixes you listed I also had to
fix the call to MPI_Allgather. More below:
> Between "size," and "caf_register_t" is a<tab> instead of a simple " ".
Fixed.
> As you mentioned yourself (in a private mail, commenting on my code):
> There is a "*" missing at the left side of the assignment lines.
Oops. Fixed.
> Wrong order for the "token" allocation: You first need to know how many
> images exists, before you can use "caf_num_images".
OOps again. Fixed.
> + MPI_Allgather (local, sizeof (void*), MPI_BYTE,
>
> There is a<tab> character between "local," and "sizeof"; it should
> have been a space.
I was trying to make the code look nicer by making things lineup. It
also helps me find typos. In general, is it ok to add spaces to make
code lineup?
> Have you tested the modifications with MPI? I mean: Compiling mpi.c,
> and then in gcc/testsuite/gfortran.dg/coarray:
Ooops. I compiled and ran the test suite but I forgot to test MPI
separately. There was a bug in the MPI_Allgather line. It turns out that
my earlier "&local" was correct after all:
MPI_Allgather (&local, sizeof (void*), MPI_BYTE,
token, sizeof (void*), MPI_BYTE, MPI_COMM_WORLD);
The first parameter of MPI_Allgather is not the data to send, but a
pointer to the data you want to send. That makes sense, otherwise you
wouldn't be able to send large data. Anyway, since what we want to send
is "local" (i.e. the address of the coarray) we need to pass a pointer
to local, i.e. &local.
Anyway, now the MPI tests all run correctly. I also recompiled and
re-ran the test suite with "make check-gfortran" and those are fine too.
As before, I can't make an automated test for memory allocation, but I
used fprintf statements to confirm that "token[caf_this_image-1]" has
the same value as "local" for the rest "registering_1.f90". Of course I
removed those for the final patch, but at least I feel confidence that
the patch does what it should.
Cheers,
Daniel.
--
I'm not overweight, I'm undertall.
[-- Attachment #2: caf-free-memory.patch --]
[-- Type: text/x-patch, Size: 4744 bytes --]
Index: libgfortran/caf/single.c
===================================================================
--- libgfortran/caf/single.c (revision 174944)
+++ libgfortran/caf/single.c (working copy)
@@ -35,7 +35,10 @@
Note: For performance reasons -fcoarry=single should be used
rather than this library. */
+/* Global variables. */
+caf_static_t *caf_static_list = NULL;
+
void
_gfortran_caf_init (int *argc __attribute__ ((unused)),
char ***argv __attribute__ ((unused)),
@@ -49,16 +52,31 @@
void
_gfortran_caf_finalize (void)
{
+ while (caf_static_list != NULL)
+ {
+ free(caf_static_list->token[0]);
+ caf_static_list = caf_static_list->prev;
+ }
}
void *
-_gfortran_caf_register (ptrdiff_t size,
- caf_register_t type __attribute__ ((unused)),
+_gfortran_caf_register (ptrdiff_t size, caf_register_t type,
void **token)
{
- *token = NULL;
- return malloc (size);
+ void *local;
+
+ local = malloc (size);
+ token = malloc (sizeof (void*) * 1);
+
+ if (type == CAF_REGTYPE_COARRAY_STATIC)
+ {
+ caf_static_t *tmp = malloc (sizeof (caf_static_t));
+ tmp->prev = caf_static_list;
+ tmp->token = token;
+ caf_static_list = tmp;
+ }
+ return local;
}
Index: libgfortran/caf/libcaf.h
===================================================================
--- libgfortran/caf/libcaf.h (revision 174944)
+++ libgfortran/caf/libcaf.h (working copy)
@@ -38,15 +38,23 @@
#define STAT_LOCKED_OTHER_IMAGE 2
#define STAT_STOPPED_IMAGE 3
-
+/* Describes what type of array we are registerring. */
typedef enum caf_register_t {
- CAF_REGTYPE_COARRAY,
+ CAF_REGTYPE_COARRAY_STATIC,
+ CAF_REGTYPE_COARRAY_ALLOC,
CAF_REGTYPE_LOCK,
CAF_REGTYPE_LOCK_COMP
}
caf_register_t;
+/* Linked list of static coarrays registered. */
+typedef struct caf_static_t {
+ void **token;
+ struct caf_static_t *prev;
+}
+caf_static_t;
+
void _gfortran_caf_init (int *, char ***, int *, int *);
void _gfortran_caf_finalize (void);
Index: libgfortran/caf/mpi.c
===================================================================
--- libgfortran/caf/mpi.c (revision 174944)
+++ libgfortran/caf/mpi.c (working copy)
@@ -42,7 +42,10 @@
static int caf_this_image;
static int caf_num_images;
+caf_static_t *caf_static_list = NULL;
+
+
/* Initialize coarray program. This routine assumes that no other
MPI initialization happened before; otherwise MPI_Initialized
had to be used. As the MPI library might modify the command-line
@@ -52,36 +55,69 @@
void
_gfortran_caf_init (int *argc, char ***argv, int *this_image, int *num_images)
{
- /* caf_mpi_initialized is only true if the main program is not written in
- Fortran. */
- MPI_Initialized (&caf_mpi_initialized);
- if (!caf_mpi_initialized)
- MPI_Init (argc, argv);
+ if (caf_num_images == 0)
+ {
+ /* caf_mpi_initialized is only true if the main program is
+ not written in Fortran. */
+ MPI_Initialized (&caf_mpi_initialized);
+ if (!caf_mpi_initialized)
+ MPI_Init (argc, argv);
- MPI_Comm_rank (MPI_COMM_WORLD, &caf_this_image);
- *this_image = ++caf_this_image;
- MPI_Comm_size (MPI_COMM_WORLD, &caf_num_images);
- *num_images = caf_num_images;
+ MPI_Comm_size (MPI_COMM_WORLD, &caf_num_images);
+ MPI_Comm_rank (MPI_COMM_WORLD, &caf_this_image);
+ caf_this_image++;
+ }
+
+ if (this_image)
+ *this_image = caf_this_image;
+ if (num_images)
+ *num_images = caf_num_images;
}
+
/* Finalize coarray program. */
void
_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;
+ }
+
if (!caf_mpi_initialized)
MPI_Finalize ();
}
void *
-_gfortran_caf_register (ptrdiff_t size,
- caf_register_t type __attribute__ ((unused)),
+_gfortran_caf_register (ptrdiff_t size, caf_register_t type,
void **token)
{
- *token = NULL;
- return malloc (size);
+ void *local;
+
+ /* Start MPI if not already started. */
+ if (caf_num_images == 0)
+ _gfortran_caf_init (NULL, NULL, NULL, NULL);
+
+ /* Token contains only a list of pointers. */
+ local = malloc (size);
+ token = malloc (sizeof (void*) * caf_num_images);
+
+ /* token[img-1] is the address of the token in image "img". */
+ MPI_Allgather (&local, sizeof (void*), MPI_BYTE,
+ token, sizeof (void*), MPI_BYTE, MPI_COMM_WORLD);
+
+ if (type == CAF_REGTYPE_COARRAY_STATIC)
+ {
+ caf_static_t *tmp = malloc (sizeof (caf_static_t));
+ tmp->prev = caf_static_list;
+ tmp->token = token;
+ caf_static_list = tmp;
+ }
+ return local;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, Fortra, Coarray] Free allocated memory for static coarrays.
2011-06-16 15:17 ` Daniel Carrera
@ 2011-06-16 23:31 ` Tobias Burnus
0 siblings, 0 replies; 4+ messages in thread
From: Tobias Burnus @ 2011-06-16 23:31 UTC (permalink / raw)
To: Daniel Carrera; +Cc: Tobias Burnus, gcc-patches, fortran
Hi Daniel,
Daniel Carrera wrote:
>> Have you tested the modifications with MPI? I mean: Compiling mpi.c,
>> and then in gcc/testsuite/gfortran.dg/coarray:
>
> Ooops. I compiled and ran the test suite but I forgot to test MPI
> separately.
Seemingly, there was also a bug in the single.c version:
+ local = malloc (size);
+ token = malloc (sizeof (void*) * 1);
I have added the missing line:
token[0] = local;
Committed the patch with this change as Rev. 175124. Thanks for the patch!
Tobias
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-06-16 23:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-16 9:45 [Patch, Fortra, Coarray] Free allocated memory for static coarrays Daniel Carrera
2011-06-16 12:55 Tobias Burnus
2011-06-16 15:17 ` Daniel Carrera
2011-06-16 23:31 ` Tobias Burnus
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).