Re: [PATCH] openacc: Warnings for strange partitioning choices for parallel regions

[Tobias Burnus <tobias@codesourcery.com>]

On 26.04.21 21:54, Thomas Schwinge wrote:
> On 2021-04-26T14:32:10+0200, Tobias Burnus <tobias@codesourcery.com> wrote:
>> first, can you add an entry regarding this flag
>> to https://gcc.gnu.org/gcc-12/changes.html ?
> Is that a standard thing that all new command-line flags do get
> highlighted there?  (I wouldn't have considered that one important
> enough?)

I think options which one expects to to use as user – especially when
they are for consideration with -W... do make sense. In general, as a
user I am happy to have a more detailed information about changes than
'something has changed'.

>> Secondly, I now see FAILs like:
>>
>> FAIL: gfortran.dg/goacc/classify-serial.f95   -O  (test for excess errors)
>> Excess errors:
>> gfortran.dg/goacc/classify-serial.f95:20:132: Warning: region contains gang partitioned code but is not gang partitioned [-Wopenacc-parallelism]
>> gfortran.dg/goacc/classify-serial.f95:20:132: Warning: region contains vector partitioned code but is not vector partitioned [-Wopenacc-parallelism]
> Eek!  I do reproduce that -- but only in a GCC build configuration
> without offloading enabled!
Aha, that explains why you did commit it like that – now I have fixed
the nonoffload version – and broken the offloaded one.
> Notice upper-case "Warning" vs. lower-case "warning".  That's for Fortran
> only; for C, C++, it's lower-case "warning" for both build variants.
>
> It's of course easy to fix up the regexp, but should we maybe rather
> figure out how to unify the reporting?
>
> I do understand that Fortran has some upper-case diagnostics, but I don't
> understand the difference/relevance of GCC build configuration without
> vs. with offloading enabled, how is that coming becoming relevant?

The default is 'global_dc = &global_diagnostic_context' which gets
overridden in fortran/error.c.

I have no idea why there is a difference but wonder whether it has
something to do with taking the LTO path (flag_generate_offload) and
some overriding of the settings.

I believe this is done in tree.c's free_lang_data:
   if (in_lto_p
       || (!flag_generate_lto && !flag_generate_offload))
...
       return 0;
     }

  /* Reset diagnostic machinery.  */
   tree_diagnostics_defaults (global_dc);

And in cgraphunit.c's symbol_table::finalize_compilation_unit
   if (!in_lto_p && g->have_offload)
     flag_generate_offload = 1;

And g->have_offload is guarded by ENABLE_OFFLOAD.

> Also notice that the preamble "In function 'MAIN__._omp_fn.0':" only
> appears in the GCC build configuration with offloading.  Looking at
> 'c-c++-common/goacc/classify-serial.c', for C, we've got "In function
> 'SERIAL._omp_fn.0':" for both GCC build configurations, and for C++,
> we've got "In function '_Z6SERIALv._omp_fn.0':" without offloading
> enabled vs. "In function 'SERIAL() [clone ._omp_fn.0]':" with offloading
> enabled.

I note that:
langhooks.c:          (context->printer, _("In function %qs"),

and in cp/error.c's cp_print_error_function:
           pp_printf (context->printer, function_category (fndecl),
                      cxx_printable_name_translate (fndecl, 2));
My bet is that here the same happens as above – the global_dc gets
reset – and the diagnostic becomes different.

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf