Re: [PATCH 1/3] Correct the reported line number in fortran combined OpenACC directives
Hi!
On Wed, 25 Jul 2018 08:53:35 -0700, Cesar Philippidis
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
Date: Sun Dec 9 12:49:20 2018 +
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
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
+
+ * trans-openmp.c (gfc_trans_oacc_combined_directive): Set the
+ location of combined acc loops.
+
2018-12-09 Thomas Schwinge
* 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
+
+ * gfortran.dg/goacc/combined-directives-3.f90: New file.
+
2018-12-09 Cesar Philippidis
* 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 ..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
Re: [PATCH 1/3] Correct the reported line number in fortran combined OpenACC directives
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. Usually this
>> isn't a problem because the fortran FE is able to report problems with
>> acc loops itself. However, there will be inaccuracies if the ME tries
>> to use those locations.
>>
>> Note that test cases are inconspicuously absent in this patch.
>> 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.
>>
>> Is this OK for trunk? I bootstrapped and regtested it on x86_64 with
>> nvptx offloading.
>>
>> Thanks,
>> Cesar
>>
>> 2018-XX-YY Cesar Philippidis
>>
>> gcc/fortran/
>> * trans-openmp.c (gfc_trans_oacc_combined_directive): Set the
>> location of combined acc loops.
>>
>> (cherry picked from gomp-4_0-branch r245653)
>>
>> diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
>> index f038f4c..e7707d0 100644
>> --- a/gcc/fortran/trans-openmp.c
>> +++ b/gcc/fortran/trans-openmp.c
>> @@ -3869,6 +3869,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)
>> {
>> @@ -3930,12 +3931,16 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
>>else
>> pushlevel ();
>>stmt = gfc_trans_omp_do (code, EXEC_OACC_LOOP, pblock, &loop_clauses,
>> NULL);
>> +
>> + 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,
Cesar
2018-XX-YY Cesar Philippidis
gcc/fortran/
* trans-openmp.c (gfc_trans_oacc_combined_directive): Set the
location of combined acc loops.
(cherry picked from gomp-4_0-branch r245653)
---
gcc/fortran/trans-openmp.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index f038f4c5bf8..b549c682533 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -3869,6 +3869,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)
{
@@ -3929,13 +3930,16 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
pblock = █
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);
}
--
2.17.1
Re: [PATCH 1/3] Correct the reported line number in fortran combined OpenACC directives
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. Usually this
> isn't a problem because the fortran FE is able to report problems with
> acc loops itself. However, there will be inaccuracies if the ME tries
> to use those locations.
>
> Note that test cases are inconspicuously absent in this patch.
> 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.
>
> Is this OK for trunk? I bootstrapped and regtested it on x86_64 with
> nvptx offloading.
>
> Thanks,
> Cesar
>
> 2018-XX-YY Cesar Philippidis
>
> gcc/fortran/
> * trans-openmp.c (gfc_trans_oacc_combined_directive): Set the
> location of combined acc loops.
>
> (cherry picked from gomp-4_0-branch r245653)
>
> diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
> index f038f4c..e7707d0 100644
> --- a/gcc/fortran/trans-openmp.c
> +++ b/gcc/fortran/trans-openmp.c
> @@ -3869,6 +3869,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)
> {
> @@ -3930,12 +3931,16 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
>else
> pushlevel ();
>stmt = gfc_trans_omp_do (code, EXEC_OACC_LOOP, pblock, &loop_clauses,
> NULL);
> +
> + if (CAN_HAVE_LOCATION_P (stmt))
> +SET_EXPR_LOCATION (stmt, loc);
This is protected_set_expr_location.
Marek
[PATCH 1/3] Correct the reported line number in fortran combined OpenACC directives
The fortran FE incorrectly records the line locations of combined acc
loop directives when it lowers the construct to gimple. Usually this
isn't a problem because the fortran FE is able to report problems with
acc loops itself. However, there will be inaccuracies if the ME tries
to use those locations.
Note that test cases are inconspicuously absent in this patch.
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.
Is this OK for trunk? I bootstrapped and regtested it on x86_64 with
nvptx offloading.
Thanks,
Cesar
2018-XX-YY Cesar Philippidis
gcc/fortran/
* trans-openmp.c (gfc_trans_oacc_combined_directive): Set the
location of combined acc loops.
(cherry picked from gomp-4_0-branch r245653)
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index f038f4c..e7707d0 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -3869,6 +3869,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)
{
@@ -3930,12 +3931,16 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
else
pushlevel ();
stmt = gfc_trans_omp_do (code, EXEC_OACC_LOOP, pblock, &loop_clauses, NULL);
+
+ if (CAN_HAVE_LOCATION_P (stmt))
+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);
}
--
2.7.4
