From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 73139 invoked by alias); 12 May 2015 22:02:49 -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 73118 invoked by uid 89); 12 May 2015 22:02:49 -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: smtp23.services.sfr.fr Received: from smtp23.services.sfr.fr (HELO smtp23.services.sfr.fr) (93.17.128.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 12 May 2015 22:02:46 +0000 Received: from filter.sfr.fr (localhost [86.72.15.254]) by msfrf2304.sfr.fr (SMTP Server) with ESMTP id 5BC9270000AA; Wed, 13 May 2015 00:02:43 +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 (254.15.72.86.rev.sfr.net [86.72.15.254]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by msfrf2304.sfr.fr (SMTP Server) with ESMTP id DB22D70000A8; Wed, 13 May 2015 00:02:42 +0200 (CEST) X-SFR-UUID: 20150512220242897.DB22D70000A8@msfrf2304.sfr.fr Message-ID: <55527874.1070602@sfr.fr> Date: Tue, 12 May 2015 22:04: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 , GCC-Patches-ML , GCC-Fortran-ML Subject: Re: [Patch, fortran, pr65548, 2nd take, v3] [5/6 Regression] gfc_conv_procedure_call References: <20150325143554.0343a7a7@vepi2> <20150402122830.4153db9b@vepi2> <551DD96F.2050706@charter.net> <20150407161152.22629ff5@vepi2> <20150429143101.1aa5d0b4@gmx.de> <20150430150728.17a76373@gmx.de> In-Reply-To: <20150430150728.17a76373@gmx.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg01166.txt.bz2 Hello, Le 30/04/2015 15:07, Andre Vehreschild a écrit : > Hi all, > > this is just a service release. I encountered that the new testcase in the > previous release included the testcase of the initial patch, that is > already on trunk. I therefore replaced the testcase allocate_with_source_5.f90 > by allocate_with_source_6.f90 (the extended testcase). Besides this there is no > difference inbetween this and the patch in: > > https://gcc.gnu.org/ml/fortran/2015-04/msg00121.html > > Sorry for the mess. For a description of the original patches scope see below. > > Bootstraps and regtests ok on x86_64-linux-gnu/F21. > > Ok for trunk? > > Regards, > Andre > > On Wed, 29 Apr 2015 14:31:01 +0200 > Andre Vehreschild wrote: > >> Hi all, >> >> after the first patch to fix the issue reported in the pr, some more issues >> were reported, which are now fixed by this new patch, aka the 2nd take. >> >> The patch modifies the gfc_trans_allocate() in order to pre-evaluate all >> source= expressions. It no longer rejects array valued source= expressions, >> but just uses gfc_conv_expr_descriptor () for most of them. Furthermore, is >> the allocate now again able to allocate arrays of strings. This feature >> previously slipped my attention. >> >> Although the reporter has not yet reported, that the patch fixes his issue, I >> like to post it for review, because there are more patches in my pipeline, >> that depend on this one. >> >> Bootstraps and regtests ok on x86_64-linux-gnu/F21. >> >> Ok, for trunk? >> questions below > > > *** trans-stmt.c 2015-05-12 14:42:17.882108651 +0200 > --- trans-stmt.c.modif 2015-05-12 14:42:11.300108561 +0200 > *************** > *** 5205,5213 **** > /* In all other cases evaluate the expr3 and create a > temporary. */ > gfc_init_se (&se, NULL); > if (code->expr3->rank != 0 > ! && code->expr3->expr_type == EXPR_FUNCTION > ! && code->expr3->value.function.isym) > gfc_conv_expr_descriptor (&se, code->expr3); > else > gfc_conv_expr_reference (&se, code->expr3); > --- 5198,5222 ---- > /* In all other cases evaluate the expr3 and create a > temporary. */ > gfc_init_se (&se, NULL); > + /* For more complicated expression, the decision when to get the > + descriptor and when to get a reference is depending on more > + conditions. The descriptor is only retrieved for functions > + that are intrinsic, elemental user-defined and known, or neither > + of the two, or are a class or type, that has a not deferred type > + array_spec. */ > if (code->expr3->rank != 0 > ! && (code->expr3->expr_type != EXPR_FUNCTION > ! || code->expr3->value.function.isym > ! || (code->expr3->value.function.esym && > ! code->expr3->value.function.esym->attr.elemental) > ! || (!code->expr3->value.function.isym > ! && !code->expr3->value.function.esym) > ! || (code->expr3->ts.type == BT_DERIVED > ! && code->expr3->ts.u.derived->as > ! && code->expr3->ts.u.derived->as->type != AS_DEFERRED) > ! || (code->expr3->ts.type == BT_CLASS > ! && CLASS_DATA (code->expr3)->as > ! && CLASS_DATA (code->expr3)->as->type != AS_DEFERRED))) > gfc_conv_expr_descriptor (&se, code->expr3); > else > gfc_conv_expr_reference (&se, code->expr3); What is the rationale for choosing between gfc_conv_expr_descriptor and gfc_conv_expr_reference? Is it contiguous array vs non-contiguous or needing an evaluation? For example why not use gfc_conv_expr_descriptor for derived type arrays? > *************** > *** 5646,5659 **** > } > else if (code->expr3->ts.type == BT_CHARACTER) > { > ! tmp = INDIRECT_REF_P (se.expr) ? > se.expr : > build_fold_indirect_ref_loc (input_location, > se.expr); > ! gfc_trans_string_copy (&block, al_len, tmp, > ! code->expr3->ts.kind, > ! expr3_len, expr3, > ! code->expr3->ts.kind); > tmp = NULL_TREE; > } > else if (al->expr->ts.type == BT_CLASS) > --- 5658,5707 ---- > } > else if (code->expr3->ts.type == BT_CHARACTER) > { > ! tree dst, src, dlen, slen; > ! /* For arrays of char arrays, a ref to the data component still > ! needs to be added, because se.expr upto now only contains the > ! descritor. */ > ! if (expr->ref && se.expr && TREE_TYPE (se.expr) != NULL_TREE > ! && GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (se.expr))) > ! { > ! dst = gfc_conv_array_data (se.expr); > ! src = gfc_conv_array_data (expr3); > ! /* For CHARACTER (len=string_length), dimension (nelems) > ! compute the total length of the string to copy. */ > ! if (nelems) > ! { > ! dlen = fold_build2_loc (input_location, MULT_EXPR, > ! size_type_node, > ! fold_convert (size_type_node, > ! se.string_length), > ! fold_convert (size_type_node, > ! nelems)); > ! slen = fold_build2_loc (input_location, MULT_EXPR, > ! size_type_node, > ! fold_convert (size_type_node, > ! expr3_len), > ! fold_convert (size_type_node, > ! nelems)); > ! } > ! else > ! { > ! dlen = se.string_length; > ! slen = expr3_len; > ! } > ! } > ! else > ! { > ! dst = INDIRECT_REF_P (se.expr) ? > se.expr : > build_fold_indirect_ref_loc (input_location, > se.expr); > ! src = expr3; > ! dlen = al_len; > ! slen = expr3_len; > ! } > ! gfc_trans_string_copy (&block, dlen, dst, code->expr3->ts.kind, > ! slen, src, code->expr3->ts.kind); > tmp = NULL_TREE; > } > else if (al->expr->ts.type == BT_CLASS) This seems to assume that the array is contiguous. Can't we just fall back to the default case for characters? The rest looks good. Mikael