From patchwork Fri Nov 27 10:16:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 1407116 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Cj9XR5csnz9s0b for ; Fri, 27 Nov 2020 21:16:54 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 07D89397243C; Fri, 27 Nov 2020 10:16:52 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa2.mentor.iphmx.com (esa2.mentor.iphmx.com [68.232.141.98]) by sourceware.org (Postfix) with ESMTPS id ABC0D385783D; Fri, 27 Nov 2020 10:16:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org ABC0D385783D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=Tobias_Burnus@mentor.com IronPort-SDR: mNjhW27iNul5s+48w0laOD6DwoIzOLaUVOF//MpZTrXUKgnSS59Tms32hogmHyWHWRS+4hI5pX 1Ty7Oytajvo7CKxxAvHJeUjHQg9azBxMTytmIORF3eCUZGZjtrnODSClMtbTOMFABLuOK4qYd8 MMG8cUwxzkf9LfgC4s7vhnevz/K0J5IK+feRFwXq9VQGFR12GpFYDdyH2DNkWuC3SE5lnk1bDF TlVrsdH7L9xLbUepFjI6Z5P3TyCfjR+QM1+97arum2XH/FV5E0SXYAZCDhznQf1lPkgv61wTsR blw= X-IronPort-AV: E=Sophos;i="5.78,374,1599552000"; d="diff'?scan'208";a="55553137" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa2.mentor.iphmx.com with ESMTP; 27 Nov 2020 02:16:47 -0800 IronPort-SDR: jNVibEiQ6zoAM++0br9mR84/38Oc1EM6fCYWzy12I5AtgHvwwZVKZENHYVoS+DnimBuIsxUbvp HFb3j0tviIaYoXzfLPAWWfN6gPsMYX1gTfP/ZyuQVWgKBwkHzX+ow3jPSM4q7aZytpWPcNhCEs XzG3cc1WrCdUDmds1Ydbrby8fwLzWDJECtK9yN2oCXjDstg8pZBLGj/duqC6/0Yt4MHNamzQQB tLpVOf3piMf9nBiISl5/voqIiiQ2UL45aGqSKdt2OH9AGPRaZxfn7d4f4XYTaGCe36PZIyE+Ng d7c= To: gcc-patches , fortran , Thomas Schwinge From: Tobias Burnus Subject: [Patch] Fortran: -fno-automatic and -fopenacc / recusion check cleanup Message-ID: <8e31f136-82c4-c923-8cc2-4c3daf2507ea@codesourcery.com> Date: Fri, 27 Nov 2020 11:16:41 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 Content-Language: en-US X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-02.mgc.mentorg.com (139.181.222.2) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Two fixes – simple, see patch + commit text. Longer description: * options: Background: - OpenMP, OpenACC and imply that a function is called concurrently and -frecursive implies recusive. In all those cases, the code may fail if a local variable is in static memory instead of stack or heap. – If a user specified 'save', we can assume/hope that they will deal with it - but with -fno-automatic (→ 'save' implied), the flags clash. - Additionally, to avoid placing local arrays in static memory, for -fopenmp/-fopenacc -frecursive and 'unlimited' stack size use for const-size arrays is implied. This patch: - Handle OpenACC as OpenMP (before it didn't imply -frecursive. * Recursive run-time check. The current code currently generates: subroutine foo() logical, save :: currently_called = .false. if (currently_called) error_stop "No recursive but called" currently_called = .true. ... ... ! Rest of code, which could indirectly call this proc again ... currently_called = .false. end This works well for recursive calls but less so for concurrency (→ OpenMP, OpenACC). As noted above, by default OpenMP/OpenACC implies -frecursive and, hence, there is no recursive check generated. The question is what code should be generated for, e.g. -fno-automatic -fopenmp or -fopenacc -fmax-stack-var-size=20 In that case, -frecursive is unset. We have two choices: - Either still always reset, which may not detect concurrent use (including recursive + concurrent use) do to a race condition. - Or avoid resetting the flag But then calling the procedure twice (e.g. beginning + end of the program) will generate a bogus error. The current code does the second – at least for -fopenmp. → PATCH: Simply use the same condition twice instead of a complicated test; do so via: 'if (recurcheckvar == NULL_TREE)'. Current code: tree recurcheckvar = NULL_TREE; ... if ((gfc_option.rtcheck & GFC_RTCHECK_RECURSION) && !is_recursive && !flag_recursive && !sym->attr.artificial) ... // declare 'recurcheckvar', generate error-message code etc. ... /* Reset recursion-check variable. */ if ((gfc_option.rtcheck & GFC_RTCHECK_RECURSION) && !is_recursive && !flag_openmp && recurcheckvar != NULL_TREE) Comments? Suggestions? – If not, I will commit it as obvious in the next days. Tobias ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter Fortran: -fno-automatic and -fopenacc / recusion check cleanup Options: -fopenmp and -fopenacc imply concurrent calls to a procedure; now also -fopenacc implies -frecursive, disabling that larger local const-size array variables use static memory. Run-time recursion check: Always reset the check variable at the end of the procedure; this avoids a bogus error with -fopenmp when called twice nonconcurrently/nonrecursively. (Issue requires using -fno-automatic or -fmax-stack-var-size= to trigger.) gcc/fortran/ChangeLog: PR fortran/98010 PR fortran/98013 * options.c (gfc_post_options): Also imply recursive with -fopenacc. * trans-decl.c (gfc_generate_function_code): Simplify condition. gcc/fortran/options.c | 16 +++++++++------- gcc/fortran/trans-decl.c | 3 +-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c index d844fa93115..66be1d586fb 100644 --- a/gcc/fortran/options.c +++ b/gcc/fortran/options.c @@ -407,32 +407,34 @@ gfc_post_options (const char **pfilename) if (!flag_automatic && flag_max_stack_var_size != -2 && flag_max_stack_var_size != 0) gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-fmax-stack-var-size=%d%>", flag_max_stack_var_size); else if (!flag_automatic && flag_recursive) gfc_warning_now (OPT_Woverwrite_recursive, "Flag %<-fno-automatic%> " "overwrites %<-frecursive%>"); - else if (!flag_automatic && flag_openmp) - gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%> implied by " - "%<-fopenmp%>"); + else if (!flag_automatic && (flag_openmp || flag_openacc)) + gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%> " + "implied by %qs", flag_openmp ? "-fopenmp" : "-fopenacc"); else if (flag_max_stack_var_size != -2 && flag_recursive) gfc_warning_now (0, "Flag %<-frecursive%> overwrites %<-fmax-stack-var-size=%d%>", flag_max_stack_var_size); - else if (flag_max_stack_var_size != -2 && flag_openmp) - gfc_warning_now (0, "Flag %<-fmax-stack-var-size=%d%> overwrites %<-frecursive%> " - "implied by %<-fopenmp%>", flag_max_stack_var_size); + else if (flag_max_stack_var_size != -2 && (flag_openmp || flag_openacc)) + gfc_warning_now (0, "Flag %<-fmax-stack-var-size=%d%> overwrites " + "%<-frecursive%> implied by %qs", flag_max_stack_var_size, + flag_openmp ? "-fopenmp" : "-fopenacc"); /* Implement -frecursive as -fmax-stack-var-size=-1. */ if (flag_recursive) flag_max_stack_var_size = -1; /* Implied -frecursive; implemented as -fmax-stack-var-size=-1. */ - if (flag_max_stack_var_size == -2 && flag_openmp && flag_automatic) + if (flag_max_stack_var_size == -2 && flag_automatic + && (flag_openmp || flag_openacc)) { flag_recursive = 1; flag_max_stack_var_size = -1; } /* Set flag_stack_arrays correctly. */ if (flag_stack_arrays == -1) flag_stack_arrays = 0; diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index 71d5c670e55..d45900a5ca5 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -6965,8 +6965,7 @@ gfc_generate_function_code (gfc_namespace * ns) gfc_init_block (&cleanup); /* Reset recursion-check variable. */ - if ((gfc_option.rtcheck & GFC_RTCHECK_RECURSION) - && !is_recursive && !flag_openmp && recurcheckvar != NULL_TREE) + if (recurcheckvar != NULL_TREE) { gfc_add_modify (&cleanup, recurcheckvar, logical_false_node); recurcheckvar = NULL;