[Patch] OpenMP requires: Fix diagnostic filename corner case

[Patch] OpenMP requires: Fix diagnostic filename corner case

[Tobias Burnus]

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

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

[-- Attachment #2: requires-loc-fix.diff --]
[-- Type: text/x-patch, Size: 1153 bytes --]

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")];

Re: [Patch] OpenMP requires: Fix diagnostic filename corner case

[Jakub Jelinek]

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