From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa2.mentor.iphmx.com (esa2.mentor.iphmx.com [68.232.141.98]) by sourceware.org (Postfix) with ESMTPS id 6D5A5386F02D; Mon, 2 Nov 2020 13:43:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 6D5A5386F02D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=Thomas_Schwinge@mentor.com IronPort-SDR: TB45SWz9t3nU3SZAMl9AjGFw+/ghepOIXkVT8zEa63i6w9eveF6nIylFRFJ/1wXMoiUDBQWKEZ k5iacr581Pb/iympGUNd5PtjJzeWDENezpysaX5kmYFSPRTAZdoVGmJqgFtmRE9TLS/L4LLf5p q/gi74ZkiR1WucMCOr16MRba86Ov3RWxgvS42Fi9CkAw15B670VnRzyPtwSooSjJB20MmqwENL kugTDQ4U4RQ8imSIwg05JKneG+4PCKvmTXyI8QpCwfpH7186lWP5nHXbMhbb6tMcx3F0GwTMbs T7U= X-IronPort-AV: E=Sophos;i="5.77,445,1596528000"; d="scan'208,223";a="54647081" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa2.mentor.iphmx.com with ESMTP; 02 Nov 2020 05:43:20 -0800 IronPort-SDR: ruhaGIQU9sU7F8X0TbBXq8gFRDzq101l57a0h1dMZlyIYzO5Vn2QWoApOz47iuRXjwvqjytd6s 8oj8WnaWEsTrz4BIdSv/QVXf3Sc+Pva8ex/eRlEfPnh3oW5ujKlRh7jp9uVCDAamnnpyxupNBf h9sJa9zUxF8cyjpYBxetM2epzH9YrY4Yy9pPg94zQrmUhcDIlKLQB2Et0o9WsyGs/RPpUrvJM3 YEnJkliY5f/LlplUzll8ZIwhlXceHeR5SH/ls6okn0/RtXWE3Kh/k2gFRYbdZx+Ed8knF8emao egA= From: Thomas Schwinge To: Tobias Burnus , , Subject: Re: [Patch, Fortran] PR 92793 - fix column used for error diagnostic In-Reply-To: References: <2f70a48d-ba2e-bc37-314b-7758a9ac1e0a@codesourcery.com> <87k0v8gfx8.fsf@euler.schwinge.homeip.net> <87ft5wgfdn.fsf@euler.schwinge.homeip.net> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/26.3 (x86_64-pc-linux-gnu) Date: Mon, 2 Nov 2020 14:43:11 +0100 Message-ID: <87o8kfg9hs.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-03.mgc.mentorg.com (139.181.222.3) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-Spam-Status: No, score=-13.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_LOTSOFHASH, RCVD_IN_DNSWL_LOW, 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: , X-List-Received-Date: Mon, 02 Nov 2020 13:43:24 -0000 --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Tobias! On 2020-10-30T12:16:05+0100, Tobias Burnus wrote: > On 30.10.20 11:47, Thomas Schwinge wrote: >>>> Fixed by introducing a new function; now one only needs to make sure >>>> that no new code will re-introduce "lb->location":-) >> ... another*existing instance* of this problem. > ... >> gfc_set_backend_locus (locus * loc) >> { >> gfc_current_backend_file =3D loc->lb->file; >> - input_location =3D loc->lb->location; >> + input_location =3D gfc_get_location (loc); >> } > > In bare usage, it seems to be fine =E2=80=93 which are 23 callers. > > However, there is additionally: > > gfc_save_backend_locus (locus * loc) > { > loc->lb =3D XCNEW (gfc_linebuf); > loc->lb->location =3D input_location; > loc->lb->file =3D gfc_current_backend_file; > } > > which is used together with: > > gfc_restore_backend_locus (locus * loc) > { > gfc_set_backend_locus (loc); > free (loc->lb); > } > > I think the latter needs to be replaced by the previous > version of "gfc_save_backend_locus" for two related reasons: > > * gfc_save_backend_locus operates with incomplete data, > i.e. loc->nextc (used by gfc_get_location) might not > be set. > * input_location might/should already contain the column > offset =E2=80=93 and you do not want to add some random offset > to it. > > Hence: LGTM =E2=80=93 if you update 'gfc_restore_backend_locus' > by inlining the previous version of 'gfc_set_backend_locus'. Thanks for the review; absolutely right, sorry for not realizing that on my own. Thusly changed, see attached, pushed "Further improve Fortran column location information [PR92793]" to master branch in commit 5677444f7e7ca15557030902c3d09dab4852fa90, and backported to releases/gcc-10 branch in commit a5c5f9e181c1f7548930f1cab91002b9d460cc92. Gr=C3=BC=C3=9Fe Thomas ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstra=C3=9Fe 201, 80634 M=C3=BCnch= en / Germany Registergericht M=C3=BCnchen HRB 106955, Gesch=C3=A4ftsf=C3=BChrer: Thomas = Heurung, Alexander Walter --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-Further-improve-Fortran-column-location-information-.patch" >From 5677444f7e7ca15557030902c3d09dab4852fa90 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Fri, 30 Oct 2020 13:13:51 +0100 Subject: [PATCH] Further improve Fortran column location information [PR92793] Building on top of commit 9c81750c5bedd7883182ee2684a012c6210ebe1d "Fortran] PR 92793 - fix column used for error diagnostic", there is another place where we have to use 'gfc_get_location' returning column-corrected locations. For example, this improves column location information for OMP constructs. gcc/fortran/ PR fortran/92793 * trans.c (gfc_set_backend_locus): Use 'gfc_get_location'. (gfc_restore_backend_locus): Adjust. gcc/testsuite/ PR fortran/92793 * gfortran.dg/goacc/pr92793-1.f90: Adjust. --- gcc/fortran/trans.c | 7 ++++-- gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 | 24 +++++++++---------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c index 8caa625ab0e8..025abe389853 100644 --- a/gcc/fortran/trans.c +++ b/gcc/fortran/trans.c @@ -1829,7 +1829,7 @@ void gfc_set_backend_locus (locus * loc) { gfc_current_backend_file = loc->lb->file; - input_location = loc->lb->location; + input_location = gfc_get_location (loc); } @@ -1839,7 +1839,10 @@ gfc_set_backend_locus (locus * loc) void gfc_restore_backend_locus (locus * loc) { - gfc_set_backend_locus (loc); + /* This only restores the information captured by gfc_save_backend_locus, + intentionally does not use gfc_get_location. */ + input_location = loc->lb->location; + gfc_current_backend_file = loc->lb->file; free (loc->lb); } diff --git a/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 b/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 index a572c6b3437b..72dd6b7b8f81 100644 --- a/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 +++ b/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 @@ -13,22 +13,22 @@ subroutine check () integer :: i, j, sum, diff !$acc parallel & - !$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information. -!$acc && ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:0\\\] #pragma acc parallel" 1 "original" } } - !$acc & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:0\\\] #pragma omp target oacc_parallel" 1 "gimple" } } + !$acc & & ! Fortran location information points to the last line, and last character of the directive. +!$acc && ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:123\\\] #pragma acc parallel" 1 "original" } } + !$acc & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:123\\\] #pragma omp target oacc_parallel" 1 "gimple" } } !$acc loop & - !$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information. - !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:0\\\] #pragma acc loop" 1 "original" } } - !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:0\\\] #pragma acc loop" 1 "gimple" } } + !$acc & & ! Fortran location information points to the last line, and last character of the directive. + !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:22\\\] #pragma acc loop" 1 "original" } } + !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:22\\\] #pragma acc loop" 1 "gimple" } } !$acc& reduction ( + : sum ) & ! { dg-line sum1 } !$acc && ! Fortran location information points to the ':' in 'reduction(+:sum)'. !$acc & & ! { dg-message "36: location of the previous reduction for 'sum'" "" { target *-*-* } sum1 } !$acc& independent do i = 1, 10 !$acc loop & -!$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information. - !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:0\\\] #pragma acc loop" 1 "original" } } - !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:0\\\] #pragma acc loop" 1 "gimple" } } +!$acc & & ! Fortran location information points to the last line, and last character of the directive. + !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:34\\\] #pragma acc loop" 1 "original" } } + !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:34\\\] #pragma acc loop" 1 "gimple" } } !$acc & reduction(-: diff ) & !$acc&reduction(- : sum) & ! { dg-line sum2 } !$acc & & ! Fortran location information points to the ':' in 'reduction(-:sum)'. @@ -38,9 +38,9 @@ subroutine check () sum & & = & & 1 - ! Fortran location information points to the last line of the statement, and there is no column location information. - ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:0\\\] sum = 1" 1 "original" } } - ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:0\\\] sum = 1" 1 "gimple" } } + ! Fortran location information points to the last line, and last character of the statement. + ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:9\\\] sum = 1" 1 "original" } } + ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:9\\\] sum = 1" 1 "gimple" } } end do end do !$acc end parallel -- 2.17.1 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-Further-improve-Fortran-column-location-informat.g10.patch" >From a5c5f9e181c1f7548930f1cab91002b9d460cc92 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Fri, 30 Oct 2020 13:13:51 +0100 Subject: [PATCH] Further improve Fortran column location information [PR92793] Building on top of commit 9c81750c5bedd7883182ee2684a012c6210ebe1d "Fortran] PR 92793 - fix column used for error diagnostic", there is another place where we have to use 'gfc_get_location' returning column-corrected locations. For example, this improves column location information for OMP constructs. gcc/fortran/ PR fortran/92793 * trans.c (gfc_set_backend_locus): Use 'gfc_get_location'. (gfc_restore_backend_locus): Adjust. gcc/testsuite/ PR fortran/92793 * gfortran.dg/goacc/pr92793-1.f90: Adjust. (cherry picked from commit 5677444f7e7ca15557030902c3d09dab4852fa90) --- gcc/fortran/trans.c | 7 ++++-- gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 | 24 +++++++++---------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c index ed0542614523..e3043d499943 100644 --- a/gcc/fortran/trans.c +++ b/gcc/fortran/trans.c @@ -1808,7 +1808,7 @@ void gfc_set_backend_locus (locus * loc) { gfc_current_backend_file = loc->lb->file; - input_location = loc->lb->location; + input_location = gfc_get_location (loc); } @@ -1818,7 +1818,10 @@ gfc_set_backend_locus (locus * loc) void gfc_restore_backend_locus (locus * loc) { - gfc_set_backend_locus (loc); + /* This only restores the information captured by gfc_save_backend_locus, + intentionally does not use gfc_get_location. */ + input_location = loc->lb->location; + gfc_current_backend_file = loc->lb->file; free (loc->lb); } diff --git a/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 b/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 index a572c6b3437b..72dd6b7b8f81 100644 --- a/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 +++ b/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 @@ -13,22 +13,22 @@ subroutine check () integer :: i, j, sum, diff !$acc parallel & - !$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information. -!$acc && ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:0\\\] #pragma acc parallel" 1 "original" } } - !$acc & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:0\\\] #pragma omp target oacc_parallel" 1 "gimple" } } + !$acc & & ! Fortran location information points to the last line, and last character of the directive. +!$acc && ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:123\\\] #pragma acc parallel" 1 "original" } } + !$acc & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:123\\\] #pragma omp target oacc_parallel" 1 "gimple" } } !$acc loop & - !$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information. - !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:0\\\] #pragma acc loop" 1 "original" } } - !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:0\\\] #pragma acc loop" 1 "gimple" } } + !$acc & & ! Fortran location information points to the last line, and last character of the directive. + !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:22\\\] #pragma acc loop" 1 "original" } } + !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:22\\\] #pragma acc loop" 1 "gimple" } } !$acc& reduction ( + : sum ) & ! { dg-line sum1 } !$acc && ! Fortran location information points to the ':' in 'reduction(+:sum)'. !$acc & & ! { dg-message "36: location of the previous reduction for 'sum'" "" { target *-*-* } sum1 } !$acc& independent do i = 1, 10 !$acc loop & -!$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information. - !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:0\\\] #pragma acc loop" 1 "original" } } - !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:0\\\] #pragma acc loop" 1 "gimple" } } +!$acc & & ! Fortran location information points to the last line, and last character of the directive. + !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:34\\\] #pragma acc loop" 1 "original" } } + !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:34\\\] #pragma acc loop" 1 "gimple" } } !$acc & reduction(-: diff ) & !$acc&reduction(- : sum) & ! { dg-line sum2 } !$acc & & ! Fortran location information points to the ':' in 'reduction(-:sum)'. @@ -38,9 +38,9 @@ subroutine check () sum & & = & & 1 - ! Fortran location information points to the last line of the statement, and there is no column location information. - ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:0\\\] sum = 1" 1 "original" } } - ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:0\\\] sum = 1" 1 "gimple" } } + ! Fortran location information points to the last line, and last character of the statement. + ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:9\\\] sum = 1" 1 "original" } } + ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:9\\\] sum = 1" 1 "gimple" } } end do end do !$acc end parallel -- 2.17.1 --=-=-=--