diff mbox series

OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant'

Message ID 5637ae04-8e34-4a30-bd7f-6dba45f3663d@baylibre.com
State New
Headers show
Series OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant' | expand

Commit Message

Tobias Burnus Feb. 13, 2024, 2:54 p.m. UTC 
Inomp_resolve_declare_variant, a code path generates a new decl for the 
base function – in doing so, it ignores the assembler name. As the 
included Fortran example shows, this will lead to a linker error. 
Solution: Also copy over the assembler name. Comments, suggestions, 
remarks before I commit it? Tobias PS: As a fallout of some testing, 
motivated by the original testcase, I have filled a couple of 
declare-variant and context-selector PRs: 113904 (dyn. 
user={condition(...)}), 113905 (multiple users of variant funcs), 113906 
(construct={...} lacks constructs).

Comments

Jakub Jelinek Feb. 13, 2024, 3:08 p.m. UTC | #1 
On Tue, Feb 13, 2024 at 03:54:28PM +0100, Tobias Burnus wrote:
> Inomp_resolve_declare_variant, a code path generates a new decl for the base
> function – in doing so, it ignores the assembler name. As the included
> Fortran example shows, this will lead to a linker error. Solution: Also copy
> over the assembler name. Comments, suggestions, remarks before I commit it?
> Tobias PS: As a fallout of some testing, motivated by the original testcase,
> I have filled a couple of declare-variant and context-selector PRs: 113904
> (dyn. user={condition(...)}), 113905 (multiple users of variant funcs),
> 113906 (construct={...} lacks constructs).

> OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant'
> 
> gcc/ChangeLog:
> 
> 	* omp-general.cc (omp_resolve_declare_variant): When building the decl
> 	for the base variant, honor also the assembler name.

That artificial function I believe should never be emitted, neither it nor
calls to it, so it shouldn't matter what DECL_ASSEMBLER_NAME it has.

Isn't all this caused just by the missing check that condition trait has a
constant expression?

IMHO that is the way to handle it in GCC 14.  Once middle-end has support
for the dynamic conditions, it again should handle it properly, never
resolve to the artificial one, but figure out what variants are possible
given the unknown state of conditions, what scores they have and depending
on that figure out what checks to perform at runtime to find out which
function should be called in each case.  It can get quite a bit complicated
with lots of possible functions and lots of different score values.

	Jakub
diff mbox series

Patch

OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant'

gcc/ChangeLog:

	* omp-general.cc (omp_resolve_declare_variant): When building the decl
	for the base variant, honor also the assembler name.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/declare-variant-20.f90: New test.

 gcc/omp-general.cc                                 |  2 +
 .../gfortran.dg/gomp/declare-variant-20.f90        | 62 ++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/gcc/omp-general.cc b/gcc/omp-general.cc
index 2e31a3f9290..bc92a170e96 100644
--- a/gcc/omp-general.cc
+++ b/gcc/omp-general.cc
@@ -2630,6 +2630,8 @@  omp_resolve_declare_variant (tree base)
       (*slot)->variants = entry.variants;
       tree alt = build_decl (DECL_SOURCE_LOCATION (base), FUNCTION_DECL,
 			     DECL_NAME (base), TREE_TYPE (base));
+      if (DECL_ASSEMBLER_NAME_SET_P (base))
+	SET_DECL_ASSEMBLER_NAME (alt, DECL_ASSEMBLER_NAME (base));
       DECL_ARTIFICIAL (alt) = 1;
       DECL_IGNORED_P (alt) = 1;
       TREE_STATIC (alt) = 1;
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-20.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-20.f90
new file mode 100644
index 00000000000..c7050a22365
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-20.f90
@@ -0,0 +1,62 @@ 
+! { dg-additional-options "-fdump-tree-gimple-asmname" }
+
+! This tests that mangled names, i.e. DECL_NAME != DECL_ASSEMBLER_NAME
+! are properly handled
+
+! This test case failed before with:
+!   undefined reference to `foo'
+! as the actual symbol is __m_MOD_foo
+
+! NOTE 1: This test relies  on late resolution of condition,
+! which is here enforced via the always_false_flag variable.
+!
+! NOTE 2: Using a variable is an OpenMP 5.1 feature that is/was not supported
+! when this test case was created, cf. PR middle-end/113904
+
+module m
+  implicit none (type, external)
+  logical :: always_false_flag = .false.
+contains
+  integer function variant1() result(res)
+    res = 1
+  end function
+
+  integer function variant2() result(res)
+    res = 2
+  end function
+
+  integer function variant3() result(res)
+    res = 3
+  end function
+
+  integer function foo() result(res)
+    !$omp  declare variant(variant1) match(construct={teams})
+    !$omp  declare variant(variant2) match(construct={parallel})
+    !$omp  declare variant(variant3) match(user={condition(always_false_flag)},construct={target})
+    res = 99
+  end
+end module m
+
+program main
+  use m
+  implicit none (type, external)
+  integer :: r1, r2, r3
+
+  r1 = foo()
+  if (r1 /= 99) stop 1
+
+  !$omp parallel if (.false.)
+    r2 = foo()
+    if (r2 /= 2) stop 2
+  !$omp end parallel
+
+  !$omp teams num_teams(1)
+    r3 = foo()
+    if (r3 /= 1) stop 3
+  !$omp end teams
+
+end program 
+
+! { dg-final { scan-tree-dump-times "r1 = __m_MOD_foo \\(\\);" 1 "gimple" } }
+! { dg-final { scan-tree-dump-times "r2 = __m_MOD_variant2 \\(\\);" 1 "gimple" } }
+! { dg-final { scan-tree-dump-times "r3 = __m_MOD_variant1 \\(\\);" 1 "gimple" } }