From patchwork Thu Apr 19 12:02:23 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 153729 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id E598CB6FE7 for ; Thu, 19 Apr 2012 22:03:24 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1335441805; h=Comment: DomainKey-Signature:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject: References:In-Reply-To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=OS953mnXziiRXugDirh+fqZYSc8=; b=hSKqg3PxV8syweM 0dBVq+MsoKYNMfL6vVbhraauycrlhMpnSAPuUKsncUr9Pq9TedNs9o5ZMYtozFMQ r/Yd3dpfmHeVbWweJpm+uUKMGwE41UtojLReWo3W4sbW2AamKY+nXOx6Au4ZiABS JyakH0VHbAmgbJ2ebXNqSaxJ0pw4= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=Jmw6mhkXpmM+5ulf/PdYXTHCyIBCksgf3bKbMahFi6vJTVcWqZhQJmYrvSs+KN twxjFGh7JOH7Rh7ALqZm0ohLhy6QZpRCCKyPkj4CAx4t0SkbdfSSZLgJvx5fq8cu lzAUsRAyv1A2Gkqj8QnhlPRw/n9VMIF6S0yHuLHIccic8=; Received: (qmail 28728 invoked by alias); 19 Apr 2012 12:03:12 -0000 Received: (qmail 28676 invoked by uid 22791); 19 Apr 2012 12:03:06 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED, RCVD_IN_DNSWL_NONE, TW_PW X-Spam-Check-By: sourceware.org Received: from mx01.qsc.de (HELO mx01.qsc.de) (213.148.129.14) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 19 Apr 2012 12:02:33 +0000 Received: from [192.168.178.22] (port-92-204-26-242.dynamic.qsc.de [92.204.26.242]) by mx01.qsc.de (Postfix) with ESMTP id E1D063D05C; Thu, 19 Apr 2012 14:02:25 +0200 (CEST) Message-ID: <4F8FFECF.2040002@net-b.de> Date: Thu, 19 Apr 2012 14:02:23 +0200 From: Tobias Burnus User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120328 Thunderbird/11.0.1 MIME-Version: 1.0 To: Mikael Morin CC: fortran@gcc.gnu.org, gcc patches Subject: Re: [Patch, Fortran] PR 52196 add -Wrealloc-lhs(-all) References: <4F3A489D.1050905@net-b.de> <201202272159.46946.mikael.morin@sfr.fr> In-Reply-To: <201202272159.46946.mikael.morin@sfr.fr> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Updated patch enclosed. On 02/14/2012 12:42 PM, Tobias Burnus wrote: > in order to gain an overview for our code whether the recent RESHAPE > (and friends) bug affects us and to determine for which assignment a > reallocation happens, useful to mitigate performance issues, I added > -Wrealloc-lhs and -Wrealloc-lhs-all. > > The flag -Wrealloc-lhs is the more useful flag: It's about arrays of > intrinsic types, which are more likely to appear in hot loops than > other types of reallocatable variables such as derived types or > (scalar) character variables with deferred length. On 02/27/2012 09:59 PM, Mikael Morin wrote: >> In turn, the warning might be printed even if at the end no realloc code is >> generated or present with -O1. > Can it be caused by the frontend not going in the realloc-lhs functions in > some cases? Especially, it seems that there is a missing codimension/coindexed > condition guarding the warning if I compare to the flag_realloc_lhs conditions > in trans-expr.c I would rather move the warnings to a function and call it in the places where we really generate the extra code, like it's done for -Warray-temporaries. Two months later I have finally worked on the patch again and followed the suggestion and added the checks to trans-expr.c. Build and regtested on x86-64-linux. OK for the trunk? Tobias PS: If you wonder about the added expr2->rank in gfc_trans_assignment_1: That check is also in gfc_alloc_allocatable_for_assignment. The only difference should be the ompws_flags value. [Recall that "array = scalar" does not cause a reallocation.] 2012-04-19 Tobias Burnus PR fortran/52196 * lang.opt (Wrealloc-lhs, Wrealloc-lhs-all): New flags. * gfortran.h (gfc_option_t): Add them. * options.c (gfc_init_options, gfc_post_options, gfc_handle_option): Handle them. * invoke.texi: Document them. * trans-expr.c (realloc_lhs_warning): New function. (gfc_trans_arrayfunc_assign, alloc_scalar_allocatable_for_assignment, gfc_trans_assignment_1): Use it. 2012-04-19 Tobias Burnus PR fortran/52196 * gfortran.dg/realloc_on_assign_14.f90: New. Index: gcc/fortran/gfortran.h =================================================================== --- gcc/fortran/gfortran.h (revision 186584) +++ gcc/fortran/gfortran.h (working copy) @@ -2219,6 +2219,8 @@ typedef struct int warn_align_commons; int warn_real_q_constant; int warn_unused_dummy_argument; + int warn_realloc_lhs; + int warn_realloc_lhs_all; int max_errors; int flag_all_intrinsics; Index: gcc/fortran/invoke.texi =================================================================== --- gcc/fortran/invoke.texi (revision 186584) +++ gcc/fortran/invoke.texi (working copy) @@ -146,9 +146,8 @@ and warnings}. -Wconversion -Wfunction-elimination -Wimplicit-interface @gol -Wimplicit-procedure -Wintrinsic-shadow -Wintrinsics-std @gol -Wline-truncation -Wno-align-commons -Wno-tabs -Wreal-q-constant @gol --Wsurprising -Wunderflow -Wunused-parameter -fmax-errors=@var{n} --fsyntax-only @gol --pedantic -pedantic-errors +-Wsurprising -Wunderflow -Wunused-parameter -Wrealloc-lhs Wrealloc-lhs-all @gol +-fmax-errors=@var{n} -fsyntax-only -pedantic -pedantic-errors } @item Debugging Options @@ -919,7 +918,24 @@ off via @option{-Wno-align-commons}. See also @opt Warn if any calls to functions are eliminated by the optimizations enabled by the @option{-ffrontend-optimize} option. +@item -Wrealloc-lhs +@opindex @code{Wrealloc-lhs} +@cindex Reallocate the LHS in assignments, notification +Warn when the compiler might insert code to for allocation or reallocation of +an allocatable array variable of intrinsic type in intrinsic assignments. In +hot loops, the Fortran 2003 reallocation feature may reduce the performance. +If the array is already allocated with the correct shape, consider using a +whole-array array-spec (e.g. @code{(:,:,:)}) for the variable on the left-hand +side to prevent the reallocation check. Note that in some cases the warning +is shown, even if the compiler will optimize reallocation checks away. For +instance, when the right-hand side contains the same variable multiplied by +a scalar. See also @option{-frealloc-lhs}. +@item -Wrealloc-lhs-all +@opindex @code{Wrealloc-lhs-all} +Warn when the compiler inserts code to for allocation or reallocation of an +allocatable variable; this includes scalars and derived types. + @item -Werror @opindex @code{Werror} @cindex warnings, to errors @@ -1561,7 +1577,8 @@ need to be in effect. The parentheses protection i @cindex Reallocate the LHS in assignments An allocatable left-hand side of an intrinsic assignment is automatically (re)allocated if it is either unallocated or has a different shape. The -option is enabled by default except when @option{-std=f95} is given. +option is enabled by default except when @option{-std=f95} is given. See +also @option{-Wrealloc-lhs}. @item -faggressive-function-elimination @opindex @code{faggressive-function-elimination} Index: gcc/fortran/lang.opt =================================================================== --- gcc/fortran/lang.opt (revision 186584) +++ gcc/fortran/lang.opt (working copy) @@ -250,6 +250,14 @@ Wreal-q-constant Fortran Warning Warn about real-literal-constants with 'q' exponent-letter +Wrealloc-lhs +Fortran Warning +Warn when a left-hand-side array variable is reallocated + +Wrealloc-lhs-all +Fortran Warning +Warn when a left-hand-side variable is reallocated + Wreturn-type Fortran Warning ; Documented in C Index: gcc/fortran/options.c =================================================================== --- gcc/fortran/options.c (revision 186584) +++ gcc/fortran/options.c (working copy) @@ -111,6 +111,8 @@ gfc_init_options (unsigned int decoded_options_cou gfc_option.warn_align_commons = 1; gfc_option.warn_real_q_constant = 0; gfc_option.warn_unused_dummy_argument = 0; + gfc_option.warn_realloc_lhs = 0; + gfc_option.warn_realloc_lhs_all = 0; gfc_option.max_errors = 25; gfc_option.flag_all_intrinsics = 0; @@ -437,6 +439,9 @@ gfc_post_options (const char **pfilename) if (gfc_option.flag_frontend_optimize == -1) gfc_option.flag_frontend_optimize = optimize; + if (gfc_option.warn_realloc_lhs_all) + gfc_option.warn_realloc_lhs = 1; + gfc_cpp_post_options (); /* FIXME: return gfc_cpp_preprocess_only (); @@ -654,6 +659,14 @@ gfc_handle_option (size_t scode, const char *arg, gfc_option.warn_line_truncation = value; break; + case OPT_Wrealloc_lhs: + gfc_option.warn_realloc_lhs = value; + break; + + case OPT_Wrealloc_lhs_all: + gfc_option.warn_realloc_lhs_all = value; + break; + case OPT_Wreturn_type: warn_return_type = value; break; Index: gcc/fortran/trans-expr.c =================================================================== --- gcc/fortran/trans-expr.c (revision 186584) +++ gcc/fortran/trans-expr.c (working copy) @@ -581,6 +581,19 @@ assign: /* End of prototype trans-class.c */ +static void +realloc_lhs_warning (bt type, bool array, locus *where) +{ + if (array && type != BT_CLASS && type != BT_DERIVED + && gfc_option.warn_realloc_lhs) + gfc_warning ("Code for reallocating the allocatable array at %L will " + "be added", where); + else if (gfc_option.warn_realloc_lhs_all) + gfc_warning ("Code for reallocating the allocatable variable at %L " + "will be added", where); +} + + static tree gfc_trans_structure_assign (tree dest, gfc_expr * expr); static void gfc_apply_interface_mapping_to_expr (gfc_interface_mapping *, gfc_expr *); @@ -6479,6 +6493,8 @@ gfc_trans_arrayfunc_assign (gfc_expr * expr1, gfc_ && !(expr2->value.function.esym && expr2->value.function.esym->result->attr.allocatable)) { + realloc_lhs_warning (expr1->ts.type, true, &expr1->where); + if (!expr2->value.function.isym) { realloc_lhs_loop_for_fcn_call (&se, &expr1->where, &ss, &loop); @@ -6740,6 +6756,8 @@ alloc_scalar_allocatable_for_assignment (stmtblock if (!expr2 || expr2->rank) return; + realloc_lhs_warning (expr2->ts.type, false, &expr2->where); + /* Since this is a scalar lhs, we can afford to do this. That is, there is no risk of side effects being repeated. */ gfc_init_se (&lse, NULL); @@ -6988,7 +7006,7 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr { /* F2003: Add the code for reallocation on assignment. */ if (gfc_option.flag_realloc_lhs - && is_scalar_reallocatable_lhs (expr1)) + && is_scalar_reallocatable_lhs (expr1)) alloc_scalar_allocatable_for_assignment (&block, rse.string_length, expr1, expr2); @@ -7031,8 +7049,10 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr if (gfc_option.flag_realloc_lhs && gfc_is_reallocatable_lhs (expr1) && !gfc_expr_attr (expr1).codimension - && !gfc_is_coindexed (expr1)) + && !gfc_is_coindexed (expr1) + && expr2->rank) { + realloc_lhs_warning (expr1->ts.type, true, &expr1->where); ompws_flags &= ~OMPWS_SCALARIZER_WS; tmp = gfc_alloc_allocatable_for_assignment (&loop, expr1, expr2); if (tmp != NULL_TREE) Index: gcc/testsuite/gfortran.dg/realloc_on_assign_14.f90 =================================================================== --- gcc/testsuite/gfortran.dg/realloc_on_assign_14.f90 (revision 0) +++ gcc/testsuite/gfortran.dg/realloc_on_assign_14.f90 (working copy) @@ -0,0 +1,39 @@ +! { dg-do compile } +! { dg-options "-Wrealloc-lhs-all -Wrealloc-lhs" } +! +! PR fortran/52196 +! +implicit none +type t + integer :: x +end type t +integer, allocatable :: a(:), b +real, allocatable :: r(:) +type(t), allocatable :: c(:) +character(len=:), allocatable :: str +character(len=:), allocatable :: astr(:) + +allocate(a(2), b, c(1)) +b = 4 ! { dg-warning "Code for reallocating the allocatable variable" } +a = [b,b] ! { dg-warning "Code for reallocating the allocatable array" } +c = [t(4)] ! { dg-warning "Code for reallocating the allocatable variable" } +a = 5 ! no realloc +c = t(5) ! no realloc +str = 'abc' ! { dg-warning "Code for reallocating the allocatable variable" } +astr = 'abc' ! no realloc +astr = ['abc'] ! { dg-warning "Code for reallocating the allocatable array" } +a = reshape(a,shape(a)) ! { dg-warning "Code for reallocating the allocatable array" } +r = sin(r) ! { dg-warning "Code for reallocating the allocatable array" } +r = sin(r(1)) ! no realloc +b = sin(r(1)) ! { dg-warning "Code for reallocating the allocatable variable" } + +a = nar() ! { dg-warning "Code for reallocating the allocatable array" } +a = nar2() ! { dg-warning "Code for reallocating the allocatable array" } +contains + function nar() + integer,allocatable :: nar(:) + end function + function nar2() + integer :: nar2(8) + end function +end