Re: [PATCH 1/3] Correct the reported line number in fortran combined OpenACC directives

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Wed, 25 Jul 2018 08:53:35 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> On 07/25/2018 08:32 AM, Marek Polacek wrote:
> > On Wed, Jul 25, 2018 at 08:29:17AM -0700, Cesar Philippidis wrote:
> >> The fortran FE incorrectly records the line locations of combined acc
> >> loop directives when it lowers the construct to gimple.

After a bit of preparational work to "use existing middle end checking
for Fortran OpenACC loop clauses"...

> >> Usually this
> >> isn't a problem because the fortran FE is able to report problems with
> >> acc loops itself.

..., the Fortran front end is no longer doing that, and...

> >> However, there will be inaccuracies if the ME tries
> >> to use those locations.
> >>
> >> Note that test cases are inconspicuously absent in this patch.

..., I've been able to verify your changes by translating your C++ test
case into Fortran.

> >> However, without this bug fix, -fopt-info-note-omp will report bogus
> >> line numbers. This code patch will be tested in a later patch in
> >> this series.

> >> +  if (CAN_HAVE_LOCATION_P (stmt))
> >> +    SET_EXPR_LOCATION (stmt, loc);
> > 
> > This is protected_set_expr_location.
> 
> Neat, thanks! This patch includes that correction. Is it ok for trunk
> after bootstrapping and regression testing?

Thanks, committed to trunk in r266924:

commit a43ff24656fa8224b249e159ea81e629ffa32664
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Sun Dec 9 12:49:20 2018 +0000

    Correct the reported line number in Fortran combined OpenACC directives
    
            gcc/fortran/
            * trans-openmp.c (gfc_trans_oacc_combined_directive): Set the
            location of combined acc loops.
            gcc/testsuite/
            * gfortran.dg/goacc/combined-directives-3.f90: New file.
    
    Reviewed-by: Thomas Schwinge <thomas@codesourcery.com>
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@266924 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/fortran/ChangeLog                              |  5 +++++
 gcc/fortran/trans-openmp.c                         |  5 +++--
 gcc/testsuite/ChangeLog                            |  4 ++++
 .../c-c++-common/goacc/combined-directives-3.c     |  1 +
 .../gfortran.dg/goacc/combined-directives-3.f90    | 26 ++++++++++++++++++++++
 5 files changed, 39 insertions(+), 2 deletions(-)

diff --git gcc/fortran/ChangeLog gcc/fortran/ChangeLog
index c6eb05174f69..e74bda7a1362 100644
--- gcc/fortran/ChangeLog
+++ gcc/fortran/ChangeLog
@@ -1,3 +1,8 @@
+2018-12-09  Cesar Philippidis  <cesar@codesourcery.com>
+
+	* trans-openmp.c (gfc_trans_oacc_combined_directive): Set the
+	location of combined acc loops.
+
 2018-12-09  Thomas Schwinge  <thomas@codesourcery.com>
 
 	* openmp.c (resolve_oacc_loop_blocks): Remove checking of OpenACC
diff --git gcc/fortran/trans-openmp.c gcc/fortran/trans-openmp.c
index c9fc4e49c450..bf3f46939e39 100644
--- gcc/fortran/trans-openmp.c
+++ gcc/fortran/trans-openmp.c
@@ -3878,6 +3878,7 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
   gfc_omp_clauses construct_clauses, loop_clauses;
   tree stmt, oacc_clauses = NULL_TREE;
   enum tree_code construct_code;
+  location_t loc = input_location;
 
   switch (code->op)
     {
@@ -3939,12 +3940,12 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
   else
     pushlevel ();
   stmt = gfc_trans_omp_do (code, EXEC_OACC_LOOP, pblock, &loop_clauses, NULL);
+  protected_set_expr_location (stmt, loc);
   if (TREE_CODE (stmt) != BIND_EXPR)
     stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0));
   else
     poplevel (0, 0);
-  stmt = build2_loc (input_location, construct_code, void_type_node, stmt,
-		     oacc_clauses);
+  stmt = build2_loc (loc, construct_code, void_type_node, stmt, oacc_clauses);
   gfc_add_expr_to_block (&block, stmt);
   return gfc_finish_block (&block);
 }
diff --git gcc/testsuite/ChangeLog gcc/testsuite/ChangeLog
index 6b26f6f510db..19bc532c9d57 100644
--- gcc/testsuite/ChangeLog
+++ gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2018-12-09  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* gfortran.dg/goacc/combined-directives-3.f90: New file.
+
 2018-12-09  Cesar Philippidis  <cesar@codesourcery.com>
 
 	* c-c++-common/goacc/combined-directives-3.c: New test.
diff --git gcc/testsuite/c-c++-common/goacc/combined-directives-3.c gcc/testsuite/c-c++-common/goacc/combined-directives-3.c
index 77d418262eac..c6e31c26a8f1 100644
--- gcc/testsuite/c-c++-common/goacc/combined-directives-3.c
+++ gcc/testsuite/c-c++-common/goacc/combined-directives-3.c
@@ -1,5 +1,6 @@
 /* Verify the accuracy of the line number associated with combined
    constructs.  */
+/* See also "../../gfortran.dg/goacc/combined-directives-3.f90".  */
 
 int
 main ()
diff --git gcc/testsuite/gfortran.dg/goacc/combined-directives-3.f90 gcc/testsuite/gfortran.dg/goacc/combined-directives-3.f90
new file mode 100644
index 000000000000..b138822827f6
--- /dev/null
+++ gcc/testsuite/gfortran.dg/goacc/combined-directives-3.f90
@@ -0,0 +1,26 @@
+! Verify the accuracy of the line number associated with combined constructs.
+! See "../../c-c++-common/goacc/combined-directives-3.c".
+
+subroutine test
+  implicit none
+  integer x, y, z
+
+  !$acc parallel loop seq auto ! { dg-error "'seq' overrides other OpenACC loop specifiers" }
+  do x = 0, 10
+     !$acc loop
+     do y = 0, 10
+     end do
+  end do
+  !$acc end parallel loop
+
+  !$acc parallel loop gang auto ! { dg-error "'auto' conflicts with other OpenACC loop specifiers" }
+  do x = 0, 10
+     !$acc loop worker auto ! { dg-error "'auto' conflicts with other OpenACC loop specifiers" }
+     do y = 0, 10
+        !$acc loop vector
+        do z = 0, 10
+        end do
+     end do
+  end do
+  !$acc end parallel loop
+end subroutine test


Grüße
 Thomas