From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17948 invoked by alias); 25 May 2015 17:47:00 -0000 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 Received: (qmail 17922 invoked by uid 89); 25 May 2015 17:47:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: smtp26.services.sfr.fr Received: from smtp26.services.sfr.fr (HELO smtp26.services.sfr.fr) (93.17.128.163) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 25 May 2015 17:46:59 +0000 Received: from filter.sfr.fr (localhost [86.72.15.252]) by msfrf2601.sfr.fr (SMTP Server) with ESMTP id 7A4841C00101A; Mon, 25 May 2015 19:46:56 +0200 (CEST) Authentication-Results: sfrmc.priv.atos.fr; dkim=none (no signature); dkim-adsp=none (no policy) header.from=mikael.morin@sfr.fr Received: from tolstoi.localhost (252.15.72.86.rev.sfr.net [86.72.15.252]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by msfrf2601.sfr.fr (SMTP Server) with ESMTP id 8B5461C001029; Mon, 25 May 2015 19:46:50 +0200 (CEST) X-SFR-UUID: 20150525174651570.8B5461C001029@msfrf2601.sfr.fr Message-ID: <55635FF8.5010900@sfr.fr> Date: Mon, 25 May 2015 18:09:00 -0000 From: Mikael Morin User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Andre Vehreschild , Paul Richard Thomas CC: "fortran@gcc.gnu.org" , gcc-patches , Damian Rouson Subject: Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram References: <20150523195252.22647b46@vepi2> <20150525122447.5aafa2da@vepi2> In-Reply-To: <20150525122447.5aafa2da@vepi2> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg02252.txt.bz2 hello Andre, Le 25/05/2015 12:24, Andre Vehreschild a écrit : > Hi Paul, > > I am not quite happy with the naming of the temporary variable. When I > initially set the prefix to "atmp" this was because the variable would be an > array most of the time and because of the number appended to it should be unique > anyway. However I would like to point out that disclosing an internal > implementation detail of the compiler to a future developer looking at the > pseudo-code dump will not help (I mean "expr3", here). I would rather use > "source" as the prefix now that I think of it with some distance to the > original naming. What do you think? > > Now that the deallocate for source's components is in the patch, I understand > why initially the source= preevaluation for derived types with allocatable > components was disallowed. Thanks for clarifying that. > > I wonder though, if we can't do better... > > Please have a look at the attached patch. It not only renames the temporary > variable from "expr3" to "source" (couldn't help, but do it. Please don't be > angry :-)), but also adds move semantics to source= expressions for the last > object to allocate. I.e., when a scalar source= expression with allocatable > components is detected, then its content is "moved" (memcpy'ed) to the last > object to allocate instead of being assigned. All former objects to allocate > are of course handled like before, i.e., components are allocated and the > contents of the source= expression is copied using the assign. But when a move > could be done the alloc/dealloc of the components is skipped. With this I hope > to safe a lot of mallocs and frees, which are not that cheap. In the most common > case where only one object is allocated, there now is only one alloc for the > components to get expr3 up and one for the object to allocate. We safe the > allocate of the allocatable components in the object to allocate and the free > of the source= components. I hope I could make clear what I desire? If not > maybe a look into the patch might help. What do you think? > > The patch of course is only a quick implementation of the idea. Please > comment, everyone! > This looks good to me as well with... *************** gfc_trans_allocate (gfc_code * code) *** 5840,5845 **** --- 5837,5856 ---- } else { + if (al->next == NULL && needs_comp_dealloc && expr->rank == 0) + { ...please use "else if" to avoid changing indentation unnecessarily. Then OK if it passes the testsuite. Give Paul a couple of days to comment, before committing. Mikael