| Message ID | e63fb3d3-50a2-8c31-9648-506fad70499e@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | OpenMP requires: Fix diagnostic filename corner case | expand |
On Fri, Jul 22, 2022 at 12:38:31PM +0200, Tobias Burnus wrote: > OpenMP requires: Fix diagnostic filename corner case > > The issue occurs when there is, e.g., main._omp_fn.0 in two files with > different OpenMP requires clauses. The function entries in the offload > table ends up having the same decl tree and, hence, the diagnostic showed > the same filename for both. Solution: Use the .o filename in this case. > > Note that the issue does not occur with same-named 'static' functions and > without the fatal error from the requires diagnostic, there would be > later a linker error due to having two 'main'. > > gcc/ > * lto-cgraph.cc (input_offload_tables): Improve requires diagnostic > when filenames come out identically. > > diff --git a/gcc/lto-cgraph.cc b/gcc/lto-cgraph.cc > index 062677a32eb..350195d86db 100644 > --- a/gcc/lto-cgraph.cc > +++ b/gcc/lto-cgraph.cc > @@ -1893,6 +1893,11 @@ input_offload_tables (bool do_force_output) > if (tmp_decl != NULL_TREE) > fn2 = IDENTIFIER_POINTER (DECL_NAME (tmp_decl)); > } > + if (fn1 == fn2) > + { > + fn1 = requires_fn; > + fn2 = file_data->file_name; > + } Ugly but ok. Jakub
OpenMP requires: Fix diagnostic filename corner case The issue occurs when there is, e.g., main._omp_fn.0 in two files with different OpenMP requires clauses. The function entries in the offload table ends up having the same decl tree and, hence, the diagnostic showed the same filename for both. Solution: Use the .o filename in this case. Note that the issue does not occur with same-named 'static' functions and without the fatal error from the requires diagnostic, there would be later a linker error due to having two 'main'. gcc/ * lto-cgraph.cc (input_offload_tables): Improve requires diagnostic when filenames come out identically. diff --git a/gcc/lto-cgraph.cc b/gcc/lto-cgraph.cc index 062677a32eb..350195d86db 100644 --- a/gcc/lto-cgraph.cc +++ b/gcc/lto-cgraph.cc @@ -1893,6 +1893,11 @@ input_offload_tables (bool do_force_output) if (tmp_decl != NULL_TREE) fn2 = IDENTIFIER_POINTER (DECL_NAME (tmp_decl)); } + if (fn1 == fn2) + { + fn1 = requires_fn; + fn2 = file_data->file_name; + } char buf1[sizeof ("unified_address, unified_shared_memory, " "reverse_offload")];
This patch addresses a corner case which causes a filename confusion: lto1: error: OpenMP 'requires' directive with 'unified_address' specified only in some compilation units lto1: note: '1.c' has 'unified_address' lto1: note: but '1.c' has not lto1: fatal error: errors during merging of translation units This happens, e.g., for a source code like the following, where two files are identical except that only one has an additional '#pragma omp requires': ---------------- int main () { int i; #pragma omp target map(from:i) i = 1; return i; } ---------------- Both have 'main._omp_fn.0' - which gets merged to the same decl such that the source file will be identical. Without the fatal error shown for requires, it would fail later during liking due to having the same symbol ('main') multiple times - but that additional diagnostic is not reached, leaving one puzzled. I did run into this with 'gcc -fopenmp test_*.c' missing that I had a second file in that dir which also started with test_... OK for mainline? Or do you think adding a testcase makes sense? Tobias PS: It might also be possible to generate this with class instantiations for valid code - other than the requires mismatch. PPS: If compiles as 'gcc -fopenmp .c' / in the testsuite, the new inform will be: lto1: note: '/tmp/ccAs0w67.o' has 'unified_address' lto1: note: but '/tmp/ccv6XCfc.o' has not That avoids the issue with showing the sane name but is not perfect, either. But that's a known issue and splittitng compiling from linking will improve the diagnostic! ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955