From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19360 invoked by alias); 19 Oct 2009 17:15:14 -0000 Received: (qmail 19286 invoked by uid 22791); 19 Oct 2009 17:15:13 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_JMF_BR X-Spam-Check-By: sourceware.org Received: from mx02.qsc.de (HELO mx02.qsc.de) (213.148.130.14) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 19 Oct 2009 17:15:06 +0000 Received: from [192.168.178.22] (port-92-204-50-213.dynamic.qsc.de [92.204.50.213]) by mx02.qsc.de (Postfix) with ESMTP id C95EB1E11E; Mon, 19 Oct 2009 19:15:03 +0200 (CEST) Message-ID: <4ADC9E95.9090000@net-b.de> Date: Mon, 19 Oct 2009 17:17:00 -0000 From: Tobias Burnus User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.4pre) Gecko/20090915 SUSE/3.0b4-2.3 Thunderbird/3.0b4 MIME-Version: 1.0 To: Janus Weil CC: gfortran , gcc-patches Subject: Re: [Patch, Fortran] PR 41586: Allocatable _scalars_ are never auto-deallocated References: <854832d40910190750w4a629bdcsb3e44652c10f99a8@mail.gmail.com> In-Reply-To: <854832d40910190750w4a629bdcsb3e44652c10f99a8@mail.gmail.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: 2009-10/txt/msg01222.txt.bz2 On 10/19/2009 04:50 PM, Janus Weil wrote: > One aside, however: There is a strange thing in the dump for case 'c' > (not necessarily connected to this patch): > > struct t1 m; > > m.j = 0B; > { > struct t1 D.1388; > struct t1 t1.0; > > t1.0.j = 0B; > D.1388 = m; > m = t1.0; > if (D.1388.j != 0B) > __builtin_free ((void *) D.1388.j); > D.1388.j = 0B; > } > > If I'm not mistaken, this code was generated by "gfc_init_default_dt" > to handle the default initialization of the variable 'm'. But it's not > quite clear to me why we need two temporaries here, or why this > 'D.1388' needs to be freed although it's barely used at all (and > certainly not allocated). Maybe someone can enlighten me on this > matter. > I think one does not need two temporaries - maybe one could get rid of one. (gfortran generates in my opinion too many temporaries. While in principle -O3 should remove them, this does not always works as PR 41494 shows. Besides, in terms of dump readability and compile memory/time, fewer temporaries are useful.) What the code does is something like: if(allocated(m%j)) then deallocate(m%j) ! Free if already allocated end if nullify(m%j) ! apply default initializer which makes sense if "m" is INTENT(OUT) in order not too leak memory. > The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk? > - if (sym->ts.type == BT_DERIVED && sym->ts.u.derived->attr.alloc_comp) + /* Remember this variable for allocation/cleanup. */ + if (sym->attr.dimension || sym->attr.allocatable + || (sym->ts.type == BT_CLASS && + (sym->ts.u.derived->components->attr.dimension + || sym->ts.u.derived->components->attr.allocatable))) gfc_defer_symbol_init (sym); + else if (sym->ts.type == BT_DERIVED && sym->ts.u.derived->attr.alloc_comp) + gfc_defer_symbol_init (sym); I would prefer if you could add the condition of "else if" as "||" to the first block: + if (sym->attr.dimension || sym->attr.allocatable + || (sym->ts.type == BT_CLASS && + (sym->ts.u.derived->components->attr.dimension + || sym->ts.u.derived->components->attr.allocatable)) + || (sym->ts.type == BT_DERIVED && sym->ts.u.derived->attr.alloc_comp)) gfc_defer_symbol_init (sym); I think that's more readable - otherwise one thinks that for BT_DERIVED a different action will be done - and as another "||" does not require more indention, I think it also does not decrease the readability. Looks OK otherwise. Thanks for the patch! Tobias > 2009-10-19 Janus Weil > > PR fortran/41586 > * parse.c (parse_derived): Correctly set 'alloc_comp' and 'pointer_comp' > for CLASS variables. > * trans-array.c (structure_alloc_comps): Handle deallocation and > nullification of allocatable scalar components. > * trans-decl.c (gfc_get_symbol_decl): Remember allocatable scalars for > automatic deallocation. > (gfc_trans_deferred_vars): Automatically deallocate allocatable scalars. > > > 2009-10-19 Janus Weil > > PR fortran/41586 > * gfortran.dg/auto_dealloc_1.f90: New test case. >