From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18993 invoked by alias); 1 Feb 2016 22:07:51 -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 18972 invoked by uid 89); 1 Feb 2016 22:07:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=BAYES_00,KAM_STOCKGEN,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=no version=3.3.2 spammy=H*F:D*sfr.fr, intermediary, 44946, sk:array_o X-Spam-User: qpsmtpd, 2 recipients X-HELO: smtp22.services.sfr.fr Received: from smtp22.services.sfr.fr (HELO smtp22.services.sfr.fr) (93.17.128.13) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 01 Feb 2016 22:07:48 +0000 Received: from filter.sfr.fr (localhost [46.193.167.111]) by msfrf2214.sfr.fr (SMTP Server) with ESMTP id 0F43770000BB; Mon, 1 Feb 2016 23:07:45 +0100 (CET) Authentication-Results: sfrmc.priv.atos.fr; dkim=none (no signature); dkim-adsp=none (no policy) header.from=mikael.morin@sfr.fr Received: from [10.188.164.151] (ppp-seco21th2-46-193-167-111.wb.wifirst.net [46.193.167.111]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by msfrf2214.sfr.fr (SMTP Server) with ESMTP id 6C6CF70000B2; Mon, 1 Feb 2016 23:07:44 +0100 (CET) X-SFR-UUID: 20160201220744444.6C6CF70000B2@msfrf2214.sfr.fr From: Mikael Morin Subject: [Patch, fortran] PR66089 fix elemental dependency mishandling To: gfortran , gcc-patches Message-ID: <56AFD723.2050709@sfr.fr> Date: Mon, 01 Feb 2016 22:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary=------------030307020105060901040902 X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00085.txt.bz2 This is a multi-part message in MIME format. --------------030307020105060901040902 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 662 Hello, this is about the case c(:) = elemental_func(c(1), ...) where as a result of a trunk change, only a reference to c(1) is saved to a temporary variable, instead of its value. The fix tries to save the amount of copying as much as possible by detecting the above case. Technically through the usage of a new field needs_temporary. The patch is a variant of the one that has been on bugzilla for months. The main difference is the usage of gfc_expr_is_variable instead of the check for expr_type == EXPR_VARIABLE (the former includes pointer-returning functions as well). Regression-tested on x86_64-unknown-linux-gnu. OK for trunk? Mikael --------------030307020105060901040902 Content-Type: text/plain; charset=UTF-8; name="pr66089_v1.CL" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="pr66089_v1.CL" Content-length: 960 MjAxNi0wMi0wMSAgTWlrYWVsIE1vcmluICA8bWlrYWVsQGdjYy5nbnUub3Jn PgoKCVBSIGZvcnRyYW4vNjYwODkKCSogdHJhbnMtZXhwci5jIChleHByX2lz X3ZhcmlhYmxlLCBnZmNfZXhwcl9pc192YXJpYWJsZSk6IFJlbmFtZQoJdGhl IGZvcm1lciB0byB0aGUgbGF0dGVyIGFuZCBtYWtlIGl0IG5vbi1zdGF0aWMu ICBVcGRhdGUgY2FsbGVycy4KCSogZ2ZvcnRyYW4uaCAoZ2ZjX2V4cHJfaXNf dmFyaWFibGUpOiBOZXcgZGVjbGFyYXRpb24uCgkoc3RydWN0IGdmY19zc19p bmZvKTogQWRkIGZpZWxkIG5lZWRzX3RlbXBvcmFyeS4KCSogdHJhbnMtYXJy YXkuYyAoZ2ZjX3NjYWxhcl9lbGVtZW50YWxfYXJnX3NhdmVkX2FzX2FyZ3Vt ZW50KToKCVRpZ2h0ZW4gdGhlIGNvbmRpdGlvbiBvbiBhZ2dyZWdhdGUgZXhw cmVzc2lvbnMgd2l0aCBhIGNoZWNrCgl0aGF0IHRoZSBleHByZXNzaW9uIGlz IGEgdmFyaWFibGUgYW5kIGRvZXNuJ3QgbmVlZCBhIHRlbXBvcmFyeS4KCShn ZmNfY29udl9yZXNvbHZlX2RlcGVuZGVuY3kpOiBBZGQgaW50ZXJtZWRpYXJ5 IHJlZmVyZW5jZSB2YXJpYWJsZS4KCVNldCB0aGUgbmVlZHNfdGVtcG9yYXJ5 IGZpZWxkLgoKMjAxNi0wMi0wMSAgTWlrYWVsIE1vcmluICA8bWlrYWVsQGdj Yy5nbnUub3JnPgoKCVBSIGZvcnRyYW4vNjYwODkKCSogZ2ZvcnRyYW4uZGcv ZWxlbWVudGFsX2RlcGVuZGVuY3lfNi5mOTA6IE5ldy4K --------------030307020105060901040902 Content-Type: text/x-patch; name="pr66089_v1.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pr66089_v1.diff" Content-length: 5192 diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c index eeb688c..2ff2833 100644 --- a/gcc/fortran/trans-array.c +++ b/gcc/fortran/trans-array.c @@ -2464,10 +2464,12 @@ gfc_scalar_elemental_arg_saved_as_reference (gfc_ss_info * ss_info) return true; /* If the expression is a data reference of aggregate type, + and the data reference is not used on the left hand side, avoid a copy by saving a reference to the content. */ - if (ss_info->expr->expr_type == EXPR_VARIABLE + if (!ss_info->data.scalar.needs_temporary && (ss_info->expr->ts.type == BT_DERIVED - || ss_info->expr->ts.type == BT_CLASS)) + || ss_info->expr->ts.type == BT_CLASS) + && gfc_expr_is_variable (ss_info->expr)) return true; /* Otherwise the expression is evaluated to a temporary variable before the @@ -4461,6 +4463,7 @@ gfc_conv_resolve_dependencies (gfc_loopinfo * loop, gfc_ss * dest, gfc_ss *ss; gfc_ref *lref; gfc_ref *rref; + gfc_ss_info *ss_info; gfc_expr *dest_expr; gfc_expr *ss_expr; int nDepend = 0; @@ -4471,15 +4474,16 @@ gfc_conv_resolve_dependencies (gfc_loopinfo * loop, gfc_ss * dest, for (ss = rss; ss != gfc_ss_terminator; ss = ss->next) { - ss_expr = ss->info->expr; + ss_info = ss->info; + ss_expr = ss_info->expr; - if (ss->info->array_outer_dependency) + if (ss_info->array_outer_dependency) { nDepend = 1; break; } - if (ss->info->type != GFC_SS_SECTION) + if (ss_info->type != GFC_SS_SECTION) { if (flag_realloc_lhs && dest_expr != ss_expr @@ -4494,6 +4498,10 @@ gfc_conv_resolve_dependencies (gfc_loopinfo * loop, gfc_ss * dest, nDepend = gfc_check_dependency (dest_expr, ss_expr, false); + if (ss_info->type == GFC_SS_REFERENCE + && gfc_check_dependency (dest_expr, ss_expr, false)) + ss_info->data.scalar.needs_temporary = 1; + continue; } diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index c5ae4c5..9f5bece 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -8825,8 +8825,8 @@ gfc_trans_array_constructor_copy (gfc_expr * expr1, gfc_expr * expr2) /* Tells whether the expression is to be treated as a variable reference. */ -static bool -expr_is_variable (gfc_expr *expr) +bool +gfc_expr_is_variable (gfc_expr *expr) { gfc_expr *arg; gfc_component *comp; @@ -8839,7 +8839,7 @@ expr_is_variable (gfc_expr *expr) if (arg) { gcc_assert (expr->value.function.isym->id == GFC_ISYM_TRANSPOSE); - return expr_is_variable (arg); + return gfc_expr_is_variable (arg); } /* A data-pointer-returning function should be considered as a variable @@ -9320,7 +9320,7 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, must have its components deallocated afterwards. */ scalar_to_array = (expr2->ts.type == BT_DERIVED && expr2->ts.u.derived->attr.alloc_comp - && !expr_is_variable (expr2) + && !gfc_expr_is_variable (expr2) && expr1->rank && !expr2->rank); scalar_to_array |= (expr1->ts.type == BT_DERIVED && expr1->rank @@ -9364,7 +9364,7 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, } tmp = gfc_trans_scalar_assign (&lse, &rse, expr1->ts, - expr_is_variable (expr2) || scalar_to_array + gfc_expr_is_variable (expr2) || scalar_to_array || expr2->expr_type == EXPR_ARRAY, !(l_is_temp || init_flag) && dealloc); gfc_add_expr_to_block (&body, tmp); diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h index 3026e3b..316ee9b 100644 --- a/gcc/fortran/trans.h +++ b/gcc/fortran/trans.h @@ -210,6 +210,10 @@ typedef struct gfc_ss_info this is the symbol of the corresponding dummy argument. */ gfc_symbol *dummy_arg; tree value; + /* Tells that the scalar is a reference to a variable that might + be present on the lhs, so that we should evaluate the value + itself before the loop, not just the reference. */ + unsigned needs_temporary:1; } scalar; @@ -464,6 +468,7 @@ bool gfc_conv_ieee_arithmetic_function (gfc_se *, gfc_expr *); tree gfc_save_fp_state (stmtblock_t *); void gfc_restore_fp_state (stmtblock_t *, tree); +bool gfc_expr_is_variable (gfc_expr *); /* Does an intrinsic map directly to an external library call This is true for array-returning intrinsics, unless diff --git a/gcc/testsuite/gfortran.dg/elemental_dependency_6.f90 b/gcc/testsuite/gfortran.dg/elemental_dependency_6.f90 new file mode 100644 index 0000000..52753f1 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/elemental_dependency_6.f90 @@ -0,0 +1,28 @@ +! { dg-do run } +! +! PR fortran/66089 +! Check that we do create a temporary for C(1) below in the assignment +! to C. + + type :: t + integer :: c + end type t + + type(t), dimension(5) :: a, b, c + + a = t(1) + b = t(7) + c = t(13) + c = plus(c(1), b) +! print *, c + if (any(c%c /= 20)) call abort + +contains + + elemental function plus(lhs, rhs) + type(t), intent(in) :: lhs, rhs + type(t) :: plus + plus%c = lhs%c + rhs%c + end function plus + +end --------------030307020105060901040902--