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

[Cesar Philippidis <cesar@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

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  <cesar@codesourcery.com>
>>
>> 	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


[-- Attachment #2: 0001-Correct-the-reported-line-number-in-fortran-combined.patch --]
[-- Type: text/x-patch, Size: 1403 bytes --]

2018-XX-YY  Cesar Philippidis  <cesar@codesourcery.com>

	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 = &block;
   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