Hi Tobias!
On 2020-11-12T12:45:24+0100, Tobias Burnus <tob...@codesourcery.com> wrote:
> For code like
> !$acc kernels
> ... a lot of loops and other code
> !$acc end kernels
>
> gfortran generates
> #pragma ..._kernels
> {
> ... lot of code
> }
>
> As the PR shows, the location associated with the #pragma
> is not the 'acc kernels' line but the one near the 'acc end kernel'
> line.
>
> The reason is that first the {...} code is generated and only then
> the outer #pragma. And using input_location as location then points to
> the wrong line:
> ...
> oacc_clauses = gfc_trans_omp_clauses (...) // fine so far
> stmt = gfc_trans_omp_code (code->block->next, true); // translates {...}
> stmt = build2_loc (input_location, construct_code, ... // wrong location
>
> This patch tries to fix this in most cases; I am sure I missed some and
> others could be handled better.
I guess we eventually should get rid of *all* such uses of
'input_location' (and then 'gfc_set_backend_locus', etc., too)... ;-)
(One step at a time.) :-)
> In the testsuite, it affects two tests by moving a 'dg-message' line with
> optimized: assigned OpenACC gang loop parallelism
> from the loop-line one up to the 'kernels'-line; I think either location
> is fine.
That change is good: for current 'parloops' OpenACC 'kernels', these
diagnostics are actually expected on the 'kernels' directive line, not on
'loop's.
> The PR has a testcase (not included) which works with -fopt-info-omp-all.
Your call whether you'd like to include that one (why not?) -- but then,
I'll establish further/similar testsuite coverage with other pending
patches of mine.
> In principle, it should also have an effect on warnings (if there are
> any) and it unsurprisingly affects --fdump-tree-*-lineno.
>
> Comments, remarks, does it look good to you?
I have not verified all the details, but it conceptually looks good to
me, thanks.
Reviewed-by: Thomas Schwinge <tho...@codesourcery.com>
Will you then please backport that to releases/gcc-10 branch, too?
Grüße
Thomas
> Fortran: improve location data for OpenACC/OpenMP directives [PR97782]
>
> gcc/fortran/ChangeLog:
>
> PR fortran/97782
> * trans-openmp.c (gfc_trans_oacc_construct, gfc_trans_omp_parallel_do,
> gfc_trans_omp_parallel_do_simd, gfc_trans_omp_parallel_sections,
> gfc_trans_omp_parallel_workshare, gfc_trans_omp_sections
> gfc_trans_omp_single, gfc_trans_omp_task, gfc_trans_omp_teams
> gfc_trans_omp_target, gfc_trans_omp_target_data,
> gfc_trans_omp_workshare): Use code->loc instead of input_location
> when building the OMP_/OACC_ construct.
>
> gcc/testsuite/ChangeLog:
>
> PR fortran/97782
> * gfortran.dg/goacc/classify-kernels-unparallelized.f95: Move dg-message
> one line up.
> * gfortran.dg/goacc/classify-kernels.f95: Likewise.
>
> gcc/fortran/trans-openmp.c | 50
> +++++++++++-----------
> .../goacc/classify-kernels-unparallelized.f95 | 4 +-
> .../gfortran.dg/goacc/classify-kernels.f95 | 4 +-
> 3 files changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
> index d2559bd0c0a..6b4ad6a7050 100644
> --- a/gcc/fortran/trans-openmp.c
> +++ b/gcc/fortran/trans-openmp.c
> @@ -3922,8 +3922,8 @@ gfc_trans_oacc_construct (gfc_code *code)
> oacc_clauses = gfc_trans_omp_clauses (&block, code->ext.omp_clauses,
> code->loc, false, true);
> stmt = gfc_trans_omp_code (code->block->next, true);
> - stmt = build2_loc (input_location, construct_code, void_type_node, stmt,
> - oacc_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), construct_code,
> + void_type_node, stmt, oacc_clauses);
> gfc_add_expr_to_block (&block, stmt);
> return gfc_finish_block (&block);
> }
> @@ -5351,8 +5351,8 @@ gfc_trans_omp_parallel_do (gfc_code *code, stmtblock_t
> *pblock,
> }
> else if (TREE_CODE (stmt) != BIND_EXPR)
> stmt = build3_v (BIND_EXPR, NULL, stmt, NULL_TREE);
> - stmt = build2_loc (input_location, OMP_PARALLEL, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), OMP_PARALLEL,
> + void_type_node, stmt, omp_clauses);
> OMP_PARALLEL_COMBINED (stmt) = 1;
> gfc_add_expr_to_block (&block, stmt);
> return gfc_finish_block (&block);
> @@ -5394,8 +5394,8 @@ gfc_trans_omp_parallel_do_simd (gfc_code *code,
> stmtblock_t *pblock,
> stmt = build3_v (BIND_EXPR, NULL, stmt, NULL_TREE);
> if (flag_openmp)
> {
> - stmt = build2_loc (input_location, OMP_PARALLEL, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), OMP_PARALLEL,
> + void_type_node, stmt, omp_clauses);
> OMP_PARALLEL_COMBINED (stmt) = 1;
> }
> gfc_add_expr_to_block (&block, stmt);
> @@ -5421,8 +5421,8 @@ gfc_trans_omp_parallel_sections (gfc_code *code)
> stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0));
> else
> poplevel (0, 0);
> - stmt = build2_loc (input_location, OMP_PARALLEL, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), OMP_PARALLEL,
> + void_type_node, stmt, omp_clauses);
> OMP_PARALLEL_COMBINED (stmt) = 1;
> gfc_add_expr_to_block (&block, stmt);
> return gfc_finish_block (&block);
> @@ -5444,8 +5444,8 @@ gfc_trans_omp_parallel_workshare (gfc_code *code)
> pushlevel ();
> stmt = gfc_trans_omp_workshare (code, &workshare_clauses);
> stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0));
> - stmt = build2_loc (input_location, OMP_PARALLEL, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), OMP_PARALLEL,
> + void_type_node, stmt, omp_clauses);
> OMP_PARALLEL_COMBINED (stmt) = 1;
> gfc_add_expr_to_block (&block, stmt);
> return gfc_finish_block (&block);
> @@ -5457,6 +5457,7 @@ gfc_trans_omp_sections (gfc_code *code, gfc_omp_clauses
> *clauses)
> stmtblock_t block, body;
> tree omp_clauses, stmt;
> bool has_lastprivate = clauses->lists[OMP_LIST_LASTPRIVATE] != NULL;
> + location_t loc = gfc_get_location (&code->loc);
>
> gfc_start_block (&block);
>
> @@ -5477,8 +5478,7 @@ gfc_trans_omp_sections (gfc_code *code, gfc_omp_clauses
> *clauses)
> }
> stmt = gfc_finish_block (&body);
>
> - stmt = build2_loc (input_location, OMP_SECTIONS, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (loc, OMP_SECTIONS, void_type_node, stmt, omp_clauses);
> gfc_add_expr_to_block (&block, stmt);
>
> return gfc_finish_block (&block);
> @@ -5489,8 +5489,8 @@ gfc_trans_omp_single (gfc_code *code, gfc_omp_clauses
> *clauses)
> {
> tree omp_clauses = gfc_trans_omp_clauses (NULL, clauses, code->loc);
> tree stmt = gfc_trans_omp_code (code->block->next, true);
> - stmt = build2_loc (input_location, OMP_SINGLE, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), OMP_SINGLE,
> void_type_node,
> + stmt, omp_clauses);
> return stmt;
> }
>
> @@ -5506,8 +5506,8 @@ gfc_trans_omp_task (gfc_code *code)
> pushlevel ();
> stmt = gfc_trans_omp_code (code->block->next, true);
> stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0));
> - stmt = build2_loc (input_location, OMP_TASK, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), OMP_TASK, void_type_node,
> + stmt, omp_clauses);
> gfc_add_expr_to_block (&block, stmt);
> return gfc_finish_block (&block);
> }
> @@ -5649,8 +5649,8 @@ gfc_trans_omp_teams (gfc_code *code, gfc_omp_clauses
> *clausesa,
> if (flag_openmp)
> {
> stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0));
> - stmt = build2_loc (input_location, OMP_TEAMS, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), OMP_TEAMS,
> + void_type_node, stmt, omp_clauses);
> if (combined)
> OMP_TEAMS_COMBINED (stmt) = 1;
> }
> @@ -5753,8 +5753,8 @@ gfc_trans_omp_target (gfc_code *code)
> }
> if (flag_openmp)
> {
> - stmt = build2_loc (input_location, OMP_TARGET, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), OMP_TARGET,
> + void_type_node, stmt, omp_clauses);
> if (code->op != EXEC_OMP_TARGET)
> OMP_TARGET_COMBINED (stmt) = 1;
> cfun->has_omp_target = true;
> @@ -5815,8 +5815,8 @@ gfc_trans_omp_target_data (gfc_code *code)
> omp_clauses = gfc_trans_omp_clauses (&block, code->ext.omp_clauses,
> code->loc);
> stmt = gfc_trans_omp_code (code->block->next, true);
> - stmt = build2_loc (input_location, OMP_TARGET_DATA, void_type_node, stmt,
> - omp_clauses);
> + stmt = build2_loc (gfc_get_location (&code->loc), OMP_TARGET_DATA,
> + void_type_node, stmt, omp_clauses);
> gfc_add_expr_to_block (&block, stmt);
> return gfc_finish_block (&block);
> }
> @@ -5876,6 +5876,7 @@ gfc_trans_omp_workshare (gfc_code *code,
> gfc_omp_clauses *clauses)
> bool singleblock_in_progress = false;
> /* True if previous gfc_code in workshare construct is not workshared. */
> bool prev_singleunit;
> + location_t loc = gfc_get_location (&code->loc);
>
> code = code->block->next;
>
> @@ -5966,7 +5967,7 @@ gfc_trans_omp_workshare (gfc_code *code,
> gfc_omp_clauses *clauses)
> {
> /* Finish single block and add it to pblock. */
> tmp = gfc_finish_block (&singleblock);
> - tmp = build2_loc (input_location, OMP_SINGLE,
> + tmp = build2_loc (loc, OMP_SINGLE,
> void_type_node, tmp, NULL_TREE);
> gfc_add_expr_to_block (pblock, tmp);
> /* Add current gfc_code to pblock. */
> @@ -5982,6 +5983,7 @@ gfc_trans_omp_workshare (gfc_code *code,
> gfc_omp_clauses *clauses)
> gfc_init_block (&singleblock);
> gfc_add_expr_to_block (&singleblock, res);
> singleblock_in_progress = true;
> + loc = gfc_get_location (&code->loc);
> }
> else
> /* Add the new statement to the block. */
> @@ -5996,7 +5998,7 @@ gfc_trans_omp_workshare (gfc_code *code,
> gfc_omp_clauses *clauses)
> {
> /* Finish single block and add it to pblock. */
> tmp = gfc_finish_block (&singleblock);
> - tmp = build2_loc (input_location, OMP_SINGLE, void_type_node, tmp,
> + tmp = build2_loc (loc, OMP_SINGLE, void_type_node, tmp,
> clauses->nowait
> ? build_omp_clause (input_location, OMP_CLAUSE_NOWAIT)
> : NULL_TREE);
> diff --git
> a/gcc/testsuite/gfortran.dg/goacc/classify-kernels-unparallelized.f95
> b/gcc/testsuite/gfortran.dg/goacc/classify-kernels-unparallelized.f95
> index 08772428c4c..6cca3d6eefb 100644
> --- a/gcc/testsuite/gfortran.dg/goacc/classify-kernels-unparallelized.f95
> +++ b/gcc/testsuite/gfortran.dg/goacc/classify-kernels-unparallelized.f95
> @@ -19,8 +19,8 @@ program main
>
> call setup(a, b)
>
> - !$acc kernels copyin (a(0:n-1), b(0:n-1)) copyout (c(0:n-1))
> - do i = 0, n - 1 ! { dg-message "optimized: assigned OpenACC seq loop
> parallelism" }
> + !$acc kernels copyin (a(0:n-1), b(0:n-1)) copyout (c(0:n-1)) ! {
> dg-message "optimized: assigned OpenACC seq loop parallelism" }
> + do i = 0, n - 1
> c(i) = a(f (i)) + b(f (i))
> end do
> !$acc end kernels
> diff --git a/gcc/testsuite/gfortran.dg/goacc/classify-kernels.f95
> b/gcc/testsuite/gfortran.dg/goacc/classify-kernels.f95
> index f2c4736e111..715a983bb26 100644
> --- a/gcc/testsuite/gfortran.dg/goacc/classify-kernels.f95
> +++ b/gcc/testsuite/gfortran.dg/goacc/classify-kernels.f95
> @@ -15,8 +15,8 @@ program main
>
> call setup(a, b)
>
> - !$acc kernels copyin (a(0:n-1), b(0:n-1)) copyout (c(0:n-1))
> - do i = 0, n - 1 ! { dg-message "optimized: assigned OpenACC gang loop
> parallelism" }
> + !$acc kernels copyin (a(0:n-1), b(0:n-1)) copyout (c(0:n-1)) ! {
> dg-message "optimized: assigned OpenACC gang loop parallelism" }
> + do i = 0, n - 1
> c(i) = a(i) + b(i)
> end do
> !$acc end kernels
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
