From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7050 invoked by alias); 16 Jun 2011 12:22:11 -0000 Received: (qmail 7035 invoked by uid 22791); 16 Jun 2011 12:22:10 -0000 X-SWARE-Spam-Status: No, hits=-0.6 required=5.0 tests=AWL,BAYES_00,FAKE_REPLY_C,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from outpost1.zedat.fu-berlin.de (HELO outpost1.zedat.fu-berlin.de) (130.133.4.66) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 16 Jun 2011 12:21:56 +0000 Received: from relay1.zedat.fu-berlin.de ([130.133.4.67]) by outpost1.zedat.fu-berlin.de (Exim 4.69) with esmtp (envelope-from ) id <1QXBa6-0004if-NZ>; Thu, 16 Jun 2011 14:21:54 +0200 Received: from mx.physik.fu-berlin.de ([160.45.64.218]) by relay1.zedat.fu-berlin.de (Exim 4.69) with esmtp (envelope-from ) id <1QXBa6-00024g-JB>; Thu, 16 Jun 2011 14:21:54 +0200 Received: from lenny32.physik.fu-berlin.de ([160.45.66.36]) by mx.physik.fu-berlin.de with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1QXBa6-0007Wd-Dq; Thu, 16 Jun 2011 14:21:54 +0200 Received: from tburnus by lenny32.physik.fu-berlin.de with local (Exim 4.69 #1 (Debian)) id 1QXBa6-0008EE-BH; Thu, 16 Jun 2011 14:21:54 +0200 Date: Thu, 16 Jun 2011 12:55:00 -0000 From: Tobias Burnus To: Daniel Carrera , gcc-patches@gcc.gnu.org, fortran@gcc.gnu.org Subject: Re: [Patch, Fortra, Coarray] Free allocated memory for static coarrays. Message-ID: <20110616122153.GA31525@physik.fu-berlin.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.18 (2008-05-17) Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-06/txt/msg01242.txt.bz2 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 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 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.