* [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant'
@ 2024-02-13 14:54 Tobias Burnus
2024-02-13 15:08 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Tobias Burnus @ 2024-02-13 14:54 UTC (permalink / raw)
To: gcc-patches, Jakub Jelinek; +Cc: Paul-Antoine Arras
[-- Attachment #1: Type: text/plain, Size: 578 bytes --]
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).
[-- Attachment #2: fix-decl-var.diff --]
[-- Type: text/x-patch, Size: 3028 bytes --]
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" } }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant'
2024-02-13 14:54 [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant' Tobias Burnus
@ 2024-02-13 15:08 ` Jakub Jelinek
2024-02-13 17:31 ` [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant') Tobias Burnus
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2024-02-13 15:08 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc-patches, Paul-Antoine Arras
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant')
2024-02-13 15:08 ` Jakub Jelinek
@ 2024-02-13 17:31 ` Tobias Burnus
2024-02-13 17:48 ` Jakub Jelinek
2024-03-04 12:58 ` [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant') Thomas Schwinge
0 siblings, 2 replies; 6+ messages in thread
From: Tobias Burnus @ 2024-02-13 17:31 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches, Paul-Antoine Arras
[-- Attachment #1.1: Type: text/plain, Size: 468 bytes --]
Jakub Jelinek wrote:
> 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.
Concur – how about the following patch?
Tobias
PS: See PR113904 for follow up tasks. / Instead of '.AND.' etc. I could
have also used some more '==', '<' etc. expressions in the modified
examples (as should have Sandra in the initial version), but,
fortunately, there is at least one '=='.
[-- Attachment #2: diag-decl-var-cond.diff --]
[-- Type: text/x-patch, Size: 24734 bytes --]
OpenMP: Reject non-const 'condition' trait in Fortran
OpenMP 5.0 only permits constant expressions for the 'condition' trait
in context selectors; this is relaxed in 5.2 but not implemented. In order
to avoid wrong code, it is now rejected.
Additionally, in Fortran, 'condition' should not accept an integer
expression, which is now ensured. Additionally, as 'device_num' should be
a conforming device number, there is now a check on the value.
PR middle-end/113904
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_context_selector): Handle splitting of
OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR.
gcc/cp/ChangeLog:
* parser.cc (cp_parser_omp_context_selector): Handle splitting of
OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR.
gcc/fortran/ChangeLog:
* openmp.cc (gfc_match_omp_context_selector):
* trans-openmp.cc (gfc_trans_omp_declare_variant): Handle splitting of
OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR.
gcc/ChangeLog:
* omp-general.cc (struct omp_ts_info): Update for splitting of
OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTYDEV_NUM,BOOL}_EXPR.
* omp-selectors.h (enum omp_tp_type): Replace
OMP_TRAIT_PROPERTY_EXPR by OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR.
gcc/testsuite/ChangeLog:
* gfortran.dg/gomp/declare-variant-1.f90: Change 'condition' trait's
argument from integer to a logical expression.
* gfortran.dg/gomp/declare-variant-11.f90: Likewise.
* gfortran.dg/gomp/declare-variant-12.f90: Likewise.
* gfortran.dg/gomp/declare-variant-13.f90: Likewise.
* gfortran.dg/gomp/declare-variant-2.f90: Likewise.
* gfortran.dg/gomp/declare-variant-2a.f90: Likewise.
* gfortran.dg/gomp/declare-variant-3.f90: Likewise.
* gfortran.dg/gomp/declare-variant-4.f90: Likewise.
* gfortran.dg/gomp/declare-variant-6.f90: Likewise.
* gfortran.dg/gomp/declare-variant-8.f90: Likewise.
* gfortran.dg/gomp/declare-variant-20.f90: New test.
gcc/c/c-parser.cc | 3 +-
gcc/cp/parser.cc | 3 +-
gcc/fortran/openmp.cc | 30 ++++++++++---
gcc/fortran/trans-openmp.cc | 3 +-
gcc/omp-general.cc | 4 +-
gcc/omp-selectors.h | 3 +-
.../gfortran.dg/gomp/declare-variant-1.f90 | 4 +-
.../gfortran.dg/gomp/declare-variant-11.f90 | 4 +-
.../gfortran.dg/gomp/declare-variant-12.f90 | 12 ++---
.../gfortran.dg/gomp/declare-variant-13.f90 | 2 +-
.../gfortran.dg/gomp/declare-variant-2.f90 | 8 ++--
.../gfortran.dg/gomp/declare-variant-20.f90 | 51 ++++++++++++++++++++++
.../gfortran.dg/gomp/declare-variant-2a.f90 | 4 +-
.../gfortran.dg/gomp/declare-variant-3.f90 | 8 ++--
.../gfortran.dg/gomp/declare-variant-4.f90 | 8 ++--
.../gfortran.dg/gomp/declare-variant-6.f90 | 14 +++---
.../gfortran.dg/gomp/declare-variant-8.f90 | 2 +-
17 files changed, 119 insertions(+), 44 deletions(-)
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index c31349dae2f..3be91d666a5 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -24656,7 +24656,8 @@ c_parser_omp_context_selector (c_parser *parser, enum omp_tss_code set,
}
while (1);
break;
- case OMP_TRAIT_PROPERTY_EXPR:
+ case OMP_TRAIT_PROPERTY_DEV_NUM_EXPR:
+ case OMP_TRAIT_PROPERTY_BOOL_EXPR:
t = c_parser_expr_no_commas (parser, NULL).value;
if (t != error_mark_node)
{
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index f0c8f9c4005..68ab74d70b9 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -47984,7 +47984,8 @@ cp_parser_omp_context_selector (cp_parser *parser, enum omp_tss_code set,
}
while (1);
break;
- case OMP_TRAIT_PROPERTY_EXPR:
+ case OMP_TRAIT_PROPERTY_DEV_NUM_EXPR:
+ case OMP_TRAIT_PROPERTY_BOOL_EXPR:
/* FIXME: this is bogus, the expression need
not be constant. */
t = cp_parser_constant_expression (parser);
diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index 0af80d54fad..d8cce6922b0 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -5790,19 +5790,39 @@ gfc_match_omp_context_selector (gfc_omp_set_selector *oss)
}
while (1);
break;
- case OMP_TRAIT_PROPERTY_EXPR:
+ case OMP_TRAIT_PROPERTY_DEV_NUM_EXPR:
+ case OMP_TRAIT_PROPERTY_BOOL_EXPR:
if (gfc_match_expr (&otp->expr) != MATCH_YES)
{
gfc_error ("expected expression at %C");
return MATCH_ERROR;
}
if (!gfc_resolve_expr (otp->expr)
- || (otp->expr->ts.type != BT_LOGICAL
+ || (property_kind == OMP_TRAIT_PROPERTY_BOOL_EXPR
+ && otp->expr->ts.type != BT_LOGICAL)
+ || (property_kind == OMP_TRAIT_PROPERTY_DEV_NUM_EXPR
&& otp->expr->ts.type != BT_INTEGER)
- || otp->expr->rank != 0)
+ || otp->expr->rank != 0
+ || otp->expr->expr_type != EXPR_CONSTANT)
{
- gfc_error ("property must be constant integer or logical "
- "expression at %C");
+ if (property_kind == OMP_TRAIT_PROPERTY_BOOL_EXPR)
+ gfc_error ("property must be a constant logical expression "
+ "at %C");
+ else
+ gfc_error ("property must be a constant integer expression "
+ "at %C");
+ return MATCH_ERROR;
+ }
+ /* Device number must be conforming, which includes
+ omp_initial_device (-1) and omp_invalid_device (-4). */
+ if (property_kind == OMP_TRAIT_PROPERTY_DEV_NUM_EXPR
+ && otp->expr->expr_type == EXPR_CONSTANT
+ && mpz_sgn (otp->expr->value.integer) < 0
+ && mpz_cmp_si (otp->expr->value.integer, -1) != 0
+ && mpz_cmp_si (otp->expr->value.integer, -4) != 0)
+ {
+ gfc_error ("property must be a conforming device number "
+ "at %C");
return MATCH_ERROR;
}
break;
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index 9599521b97c..a2bf15665b3 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -8426,7 +8426,8 @@ gfc_trans_omp_declare_variant (gfc_namespace *ns)
{
switch (otp->property_kind)
{
- case OMP_TRAIT_PROPERTY_EXPR:
+ case OMP_TRAIT_PROPERTY_DEV_NUM_EXPR:
+ case OMP_TRAIT_PROPERTY_BOOL_EXPR:
{
gfc_se se;
gfc_init_se (&se, NULL);
diff --git a/gcc/omp-general.cc b/gcc/omp-general.cc
index 2e31a3f9290..2c095200d5b 100644
--- a/gcc/omp-general.cc
+++ b/gcc/omp-general.cc
@@ -1163,7 +1163,7 @@ struct omp_ts_info omp_ts_map[] =
},
{ "device_num",
(1 << OMP_TRAIT_SET_TARGET_DEVICE),
- OMP_TRAIT_PROPERTY_EXPR, false,
+ OMP_TRAIT_PROPERTY_DEV_NUM_EXPR, false,
NULL
},
{ "vendor",
@@ -1208,7 +1208,7 @@ struct omp_ts_info omp_ts_map[] =
},
{ "condition",
(1 << OMP_TRAIT_SET_USER),
- OMP_TRAIT_PROPERTY_EXPR, true,
+ OMP_TRAIT_PROPERTY_BOOL_EXPR, true,
NULL
},
{ "target",
diff --git a/gcc/omp-selectors.h b/gcc/omp-selectors.h
index 78b810d5215..c61808ec0ad 100644
--- a/gcc/omp-selectors.h
+++ b/gcc/omp-selectors.h
@@ -64,7 +64,8 @@ enum omp_tp_type {
OMP_TRAIT_PROPERTY_NONE,
OMP_TRAIT_PROPERTY_ID,
OMP_TRAIT_PROPERTY_NAME_LIST,
- OMP_TRAIT_PROPERTY_EXPR,
+ OMP_TRAIT_PROPERTY_DEV_NUM_EXPR,
+ OMP_TRAIT_PROPERTY_BOOL_EXPR,
OMP_TRAIT_PROPERTY_CLAUSE_LIST,
OMP_TRAIT_PROPERTY_EXTENSION
};
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-1.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-1.f90
index 50d7e41613f..9b68397d190 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-1.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-1.f90
@@ -20,11 +20,11 @@ module main
!$omp & match (construct={parallel,do}, &
!$omp & device={isa(avx512f,avx512vl),kind(host,cpu)}, &
!$omp & implementation={vendor(score(0):gnu),unified_shared_memory}, &
- !$omp & user={condition(score(0):0)})
+ !$omp & user={condition(score(0):.false.)})
!$omp declare variant (bar) &
!$omp & match (device={arch(x86_64,powerpc64),isa(avx512f,popcntb)}, &
!$omp & implementation={atomic_default_mem_order(seq_cst),made_up_selector("foo", 13, "bar")}, &
- !$omp & user={condition(3-3)})
+ !$omp & user={condition(.true. .AND. (.not. .true.))})
! { dg-warning "unknown selector 'made_up_selector'" "" { target *-*-* } .-2 }
end function
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-11.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-11.f90
index 3593c9a5bb3..15b6901a02e 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-11.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-11.f90
@@ -49,8 +49,8 @@ contains
subroutine f13 ()
!$omp declare variant (f10) match (device={isa("avx512f")})
- !$omp declare variant (f11) match (user={condition(1)},device={isa(avx512f)},implementation={vendor(gnu)})
- !$omp declare variant (f12) match (user={condition(2 + 1)},device={isa(avx512f)})
+ !$omp declare variant (f11) match (user={condition(.true.)},device={isa(avx512f)},implementation={vendor(gnu)})
+ !$omp declare variant (f12) match (user={condition(.true. .NEQV. .false.)},device={isa(avx512f)})
end subroutine
subroutine f14 ()
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-12.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-12.f90
index 2fd8abd0dc7..f1b4a2280ec 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-12.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-12.f90
@@ -17,7 +17,7 @@ contains
subroutine f04 ()
!$omp declare variant (f01) match (device={isa("avx512f","avx512vl")}) ! 16
!$omp declare variant (f02) match (implementation={vendor(score(15):gnu)})
- !$omp declare variant (f03) match (user={condition(score(11):1)})
+ !$omp declare variant (f03) match (user={condition(score(11):.true.)})
end subroutine
subroutine f05 ()
@@ -32,7 +32,7 @@ contains
subroutine f08 ()
!$omp declare variant (f05) match (device={isa(avx512f,avx512vl)}) ! 16
!$omp declare variant (f06) match (implementation={vendor(score(15):gnu)})
- !$omp declare variant (f07) match (user={condition(score(17):1)})
+ !$omp declare variant (f07) match (user={condition(score(17):.true.)})
end subroutine
subroutine f09 ()
@@ -48,7 +48,7 @@ contains
end subroutine
subroutine f13 ()
- !$omp declare variant (f09) match (device={arch(x86_64)},user={condition(score(65):1)}) ! 64+65
+ !$omp declare variant (f09) match (device={arch(x86_64)},user={condition(score(65):.true.)}) ! 64+65
!$omp declare variant (f10) match (implementation={vendor(score(127):"gnu")})
!$omp declare variant (f11) match (device={isa(ssse3)}) ! 128
!$omp declare variant (f12) match (implementation={atomic_default_mem_order(score(126):seq_cst)})
@@ -65,7 +65,7 @@ contains
subroutine f17 ()
!$omp declare variant (f14) match (construct={teams,parallel,do}) ! 16+8+4
- !$omp declare variant (f15) match (construct={parallel},user={condition(score(19):1)}) ! 8+19
+ !$omp declare variant (f15) match (construct={parallel},user={condition(score(19):.true.)}) ! 8+19
!$omp declare variant (f16) match (implementation={atomic_default_mem_order(score(27):seq_cst)})
end subroutine
@@ -80,7 +80,7 @@ contains
subroutine f21 ()
!$omp declare variant (f18) match (construct={teams,parallel,do}) ! 16+8+4
- !$omp declare variant (f19) match (construct={do},user={condition(score(25):1)}) ! 4+25
+ !$omp declare variant (f19) match (construct={do},user={condition(score(25):.true.)}) ! 4+25
!$omp declare variant (f20) match (implementation={atomic_default_mem_order(score(28):seq_cst)})
end subroutine
@@ -110,7 +110,7 @@ contains
subroutine f29 ()
!$omp declare variant (f26) match (construct={parallel,do}) ! 2+1
- !$omp declare variant (f27) match (construct={do},user={condition(1)}) ! 4
+ !$omp declare variant (f27) match (construct={do},user={condition(.true.)}) ! 4
!$omp declare variant (f28) match (implementation={atomic_default_mem_order(score(3):seq_cst)})
end subroutine
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-13.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-13.f90
index 91648f9bcf4..97484a63d0b 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-13.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-13.f90
@@ -30,7 +30,7 @@ contains
!$omp declare variant (f01) match (device={isa("avx512f")}) ! 4 or 8
!$omp declare variant (f02) match (implementation={vendor(score(3):gnu)},device={kind(cpu)}) ! (1 or 2) + 3
- !$omp declare variant (f03) match (user={condition(score(9):1)})
+ !$omp declare variant (f03) match (user={condition(score(9):.true.)})
!$omp declare variant (f04) match (implementation={vendor(score(6):gnu)},device={kind(host)}) ! (1 or 2) + 6
f05 = x
end function
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-2.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-2.f90
index cbb29f87302..7fc5071feff 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-2.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-2.f90
@@ -15,7 +15,7 @@ contains
!$omp declare variant () ! { dg-error "" }
end subroutine
subroutine f5 ()
- !$omp declare variant match(user={condition(0)}) ! { dg-error "expected '\\(' at .1." }
+ !$omp declare variant match(user={condition(.false.)}) ! { dg-error "expected '\\(' at .1." }
end subroutine
subroutine f6 ()
!$omp declare variant (f1) ! { dg-error "expected 'match' at .1." }
@@ -66,7 +66,7 @@ contains
!$omp declare variant (f1) match(user={condition(f1)}) ! { dg-error "expected expression at .1." }
end subroutine
subroutine f22 ()
- !$omp declare variant (f1) match(user={condition(1, 2, 3)}) ! { dg-error "expected '\\)' at .1." }
+ !$omp declare variant (f1) match(user={condition(.false., .true., .false.)}) ! { dg-error "expected '\\)' at .1." }
end subroutine
subroutine f23 ()
!$omp declare variant (f1) match(construct={master}) ! { dg-warning "unknown selector 'master' for context selector set 'construct'" }
@@ -189,9 +189,9 @@ contains
!$omp declare variant (f1) match(implementation={atomic_default_mem_order("relaxed")}) ! { dg-error "expected identifier at .1." }
end subroutine
subroutine f77 ()
- !$omp declare variant (f1) match(user={condition(score(f76):1)}) ! { dg-error ".score. argument must be constant integer expression at .1." }
+ !$omp declare variant (f1) match(user={condition(score(f76):.true.)}) ! { dg-error ".score. argument must be constant integer expression at .1." }
end subroutine
subroutine f78 ()
- !$omp declare variant (f1) match(user={condition(score(-130):1)}) ! { dg-error ".score. argument must be non-negative" }
+ !$omp declare variant (f1) match(user={condition(score(-130):.true.)}) ! { dg-error ".score. argument must be non-negative" }
end subroutine
end module
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..17fdcb7e8bc
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-20.f90
@@ -0,0 +1,51 @@
+! PR middle-end/113904
+
+module m
+ implicit none (type, external)
+ logical, parameter :: parameter_true = .false.
+ logical :: false_flag = .false.
+ integer :: my_dev_num
+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 variant4() result(res)
+ res = 4
+ end function
+
+ integer function variant5() result(res)
+ res = 4
+ end function
+
+ integer function variant6() result(res)
+ res = 4
+ end function
+
+ integer function foo() result(res)
+ ! 'condition'
+ !$omp declare variant(variant1) match(user={condition(parameter_true)},construct={teams}) ! OK
+ ! Below: OK since OpenMP 5.1 - but not yet supported: PR middle-end/113904
+ !$omp declare variant(variant2) match(user={condition(false_flag)},construct={parallel}) ! { dg-error "property must be a constant logical expression" }
+ !$omp declare variant(variant3) match(user={condition(1)},construct={target}) ! { dg-error "property must be a constant logical expression" }
+
+ ! 'device_num'
+ !$omp declare variant(variant4) match(target_device={device_num(0)}) ! OK
+ !$omp declare variant(variant4) match(target_device={device_num(2)}) ! OK - assuming there are two non-host devices.
+ !$omp declare variant(variant5) match(target_device={device_num(-1)}) ! OK - omp_initial_device
+ !$omp declare variant(variant5) match(target_device={device_num(-4)}) ! OK - omp_invalid_device (will never match)
+ ! OK - but not handled -> PR middle-end/113904
+ !$omp declare variant(variant5) match(target_device={device_num(my_device)}) ! { dg-error "property must be a constant integer expression" }
+ !$omp declare variant(variant5) match(target_device={device_num(-2)}) ! { dg-error "property must be a conforming device number" }
+
+ res = 99
+ end
+end module m
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-2a.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-2a.f90
index edc9b27f884..b44322ac02a 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-2a.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-2a.f90
@@ -10,10 +10,10 @@ contains
!$omp declare variant (f1) match(construct={parallel},construct={parallel}) ! { dg-error "selector set 'construct' specified more than once" }
end subroutine
subroutine f30 ()
- !$omp declare variant (f1) match(user={condition(0)},construct={target},user={condition(0)}) ! { dg-error "selector set 'user' specified more than once" }
+ !$omp declare variant (f1) match(user={condition(.false.)},construct={target},user={condition(.false.)}) ! { dg-error "selector set 'user' specified more than once" }
end subroutine
subroutine f31 ()
- !$omp declare variant (f1) match(user={condition(0)},user={condition(1)}) ! { dg-error "selector set 'user' specified more than once" }
+ !$omp declare variant (f1) match(user={condition(.false.)},user={condition(.true.)}) ! { dg-error "selector set 'user' specified more than once" }
end subroutine
subroutine f37 ()
!$omp declare variant (f1) match(device={kind(unknown)}) ! { dg-warning "unknown property 'unknown' of 'kind' selector" }
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-3.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-3.f90
index c62622b607b..6b23d40e410 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-3.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-3.f90
@@ -210,13 +210,13 @@ contains
!$omp& vendor(score(22):gnu),unified_address,extension(score(22):foobar)}) ! { dg-warning "unknown property 'foobar' of 'extension' selector" "" { target *-*-* } .-1 }
end subroutine
subroutine f72 ()
- !$omp declare variant (f13) match (user={condition(0)})
+ !$omp declare variant (f13) match (user={condition(.false.)})
end subroutine
subroutine f73 ()
- !$omp declare variant (f13) match (user={condition(272-272*1)})
+ !$omp declare variant (f13) match (user={condition(.true..and..not..true.)})
end subroutine
subroutine f74 ()
- !$omp declare variant (f13) match (user={condition(score(25):1)})
+ !$omp declare variant (f13) match (user={condition(score(25):.true.)})
end subroutine
subroutine f75 ()
!$omp declare variant (f13) match (device={kind(any,"any")})
@@ -231,7 +231,7 @@ contains
!$omp declare variant (f13) match (implementation={vendor(nvidia)})
end subroutine
subroutine f79 ()
- !$omp declare variant (f13) match (user={condition(score(0):0)})
+ !$omp declare variant (f13) match (user={condition(score(0):.false.)})
end subroutine
end module
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-4.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-4.f90
index bc4f41647b4..5c7fee23504 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-4.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-4.f90
@@ -44,10 +44,10 @@ contains
end function
end interface
- !$omp declare variant (f1) match (user={condition(1)})
- !$omp declare variant (f2) match (user={condition(score(1):1)})
- !$omp declare variant (f3) match (user={condition(score(3):1)})
- !$omp declare variant (f4) match (user={condition(score(2):1)})
+ !$omp declare variant (f1) match (user={condition(.true.)})
+ !$omp declare variant (f2) match (user={condition(score(1):.true.)})
+ !$omp declare variant (f3) match (user={condition(score(3):.true.)})
+ !$omp declare variant (f4) match (user={condition(score(2):.true.)})
!$omp declare variant (f5) match (implementation={vendor(gnu)})
f6 = z + x + y
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-6.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-6.f90
index 3f33f38b9bc..63a8bd8744a 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-6.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-6.f90
@@ -24,7 +24,7 @@ contains
integer, intent(in) :: x
integer (kind = 8), intent(in) :: y
real :: z
- !$omp declare variant (f1) match (user={condition(0)},construct={parallel})
+ !$omp declare variant (f1) match (user={condition(.false.)},construct={parallel})
f3 = 0.0
end function
@@ -33,7 +33,7 @@ contains
integer, intent(in) :: x
integer (kind = 8), intent(in) :: y
real :: z
- !$omp declare variant (f1) match (construct={parallel},user={condition(score(1):1)})
+ !$omp declare variant (f1) match (construct={parallel},user={condition(score(1):.true.)})
f4 = 0.0
end function
@@ -50,7 +50,7 @@ contains
integer, intent(in) :: x
integer (kind = 8), intent(in) :: y
real :: z
- !$omp declare variant (f5) match (user={condition(0)}) ! { dg-error "'f5' used as a variant with incompatible 'construct' selector sets" }
+ !$omp declare variant (f5) match (user={condition(.false.)}) ! { dg-error "'f5' used as a variant with incompatible 'construct' selector sets" }
f6 = 0.0
end function
@@ -59,7 +59,7 @@ contains
integer, intent(in) :: x
integer (kind = 8), intent(in) :: y
real :: z
- !$omp declare variant (f5) match (construct={parallel},user={condition(score(1):1)})
+ !$omp declare variant (f5) match (construct={parallel},user={condition(score(1):.true.)})
f7 = 0.0
end function
@@ -76,7 +76,7 @@ contains
integer, intent(in) :: x
integer (kind = 8), intent(in) :: y
real :: z
- !$omp declare variant (f8) match (user={condition(0)},construct={do}) ! { dg-error "'f8' used as a variant with incompatible 'construct' selector sets" }
+ !$omp declare variant (f8) match (user={condition(.false.)},construct={do}) ! { dg-error "'f8' used as a variant with incompatible 'construct' selector sets" }
f9 = 0.0
end function
@@ -85,7 +85,7 @@ contains
integer, intent(in) :: x
integer (kind = 8), intent(in) :: y
real :: z
- !$omp declare variant (f8) match (user={condition(1)})
+ !$omp declare variant (f8) match (user={condition(.true.)})
f10 = 0.0
end function
@@ -111,7 +111,7 @@ contains
integer, intent(in) :: x
integer (kind = 8), intent(in) :: y
real :: z
- !$omp declare variant (f11) match (user={condition(score(1):1)},construct={target,teams,parallel,do}) ! { dg-error "'f11' used as a variant with incompatible 'construct' selector sets" }
+ !$omp declare variant (f11) match (user={condition(score(1):.true.)},construct={target,teams,parallel,do}) ! { dg-error "'f11' used as a variant with incompatible 'construct' selector sets" }
f13 = 0.0
end function
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-8.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-8.f90
index c751489a5db..d69e552eeb7 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-variant-8.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-8.f90
@@ -23,7 +23,7 @@ contains
end subroutine
subroutine f06 ()
- !$omp declare variant (f05) match (user={condition(1)},implementation={atomic_default_mem_order(relaxed)})
+ !$omp declare variant (f05) match (user={condition(.true.)},implementation={atomic_default_mem_order(relaxed)})
end subroutine
subroutine f07 ()
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant')
2024-02-13 17:31 ` [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant') Tobias Burnus
@ 2024-02-13 17:48 ` Jakub Jelinek
2024-02-13 20:03 ` [Patch] OpenMP: Reject non-const 'condition' trait in Fortran Tobias Burnus
2024-03-04 12:58 ` [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant') Thomas Schwinge
1 sibling, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2024-02-13 17:48 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc-patches, Paul-Antoine Arras
On Tue, Feb 13, 2024 at 06:31:02PM +0100, Tobias Burnus wrote:
> PR middle-end/113904
>
> gcc/c/ChangeLog:
>
> * c-parser.cc (c_parser_omp_context_selector): Handle splitting of
> OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR.
>
> gcc/cp/ChangeLog:
>
> * parser.cc (cp_parser_omp_context_selector): Handle splitting of
> OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR.
>
> gcc/fortran/ChangeLog:
>
> * openmp.cc (gfc_match_omp_context_selector):
> * trans-openmp.cc (gfc_trans_omp_declare_variant): Handle splitting of
> OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR.
>
>
> gcc/ChangeLog:
>
> * omp-general.cc (struct omp_ts_info): Update for splitting of
> OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTYDEV_NUM,BOOL}_EXPR.
_{ missing above before DEV_NUM
> * omp-selectors.h (enum omp_tp_type): Replace
> OMP_TRAIT_PROPERTY_EXPR by OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR.
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/gomp/declare-variant-1.f90: Change 'condition' trait's
> argument from integer to a logical expression.
> * gfortran.dg/gomp/declare-variant-11.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-12.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-13.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-2.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-2a.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-3.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-4.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-6.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-8.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-20.f90: New test.
Otherwise LGTM.
Of course it makes me wonder to what extent we actually do support the
OpenMP 5.1 target_device device_num trait with constant or non-constant
device num, because similarly to the user condition trait if it something
that can't be decided at compile time but needs to be checked at runtime,
we need to be able to compute scores of all the variants that could be
effective depending on the device_num comparison and/or condition
evaluation and turn that into set of runtime comparisons with choices of
the variants depending on the scores.
If one is lucky, it can be device == device_num ? fn1 : fn2, but it can
be more complex than that.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] OpenMP: Reject non-const 'condition' trait in Fortran
2024-02-13 17:48 ` Jakub Jelinek
@ 2024-02-13 20:03 ` Tobias Burnus
0 siblings, 0 replies; 6+ messages in thread
From: Tobias Burnus @ 2024-02-13 20:03 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches, Paul-Antoine Arras
[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]
Hi Jakub,
Jakub Jelinek wrote:
> Of course it makes me wonder to what extent we actually do support the
> OpenMP 5.1 target_device device_num trait with constant or non-constant
> device num:
Answer: If one removes some early errors such that the compiler
continues a bit further, one gets:
36 | !$omp declare variant(variant4) match(target_device={device_num(0)}) ! OK
| 1
sorry, unimplemented: 'target_device' selector set is not supported yet
Hence: Not yet supported, but the Sandra added the basic parsing
support for it to GCC 14 when she improved the handling.
However, the review-pending metadirective patch set
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/642005.html
has
[PATCH 3/8] libgomp: runtime support for target_device selector
Thus, once we GCC 15 development work has started, we can look into
this feature and some other patches ...
Other pending patches:https://gcc.gnu.org/wiki/openmpPendingPatches
Tobias
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant')
2024-02-13 17:31 ` [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant') Tobias Burnus
2024-02-13 17:48 ` Jakub Jelinek
@ 2024-03-04 12:58 ` Thomas Schwinge
1 sibling, 0 replies; 6+ messages in thread
From: Thomas Schwinge @ 2024-03-04 12:58 UTC (permalink / raw)
To: Tobias Burnus; +Cc: Jakub Jelinek, gcc-patches, Paul-Antoine Arras
Hi Tobias!
On 2024-02-13T18:31:02+0100, Tobias Burnus <tburnus@baylibre.com> wrote:
> --- a/gcc/fortran/openmp.cc
> +++ b/gcc/fortran/openmp.cc
> + /* Device number must be conforming, which includes
> + omp_initial_device (-1) and omp_invalid_device (-4). */
> + if (property_kind == OMP_TRAIT_PROPERTY_DEV_NUM_EXPR
> + && otp->expr->expr_type == EXPR_CONSTANT
> + && mpz_sgn (otp->expr->value.integer) < 0
> + && mpz_cmp_si (otp->expr->value.integer, -1) != 0
> + && mpz_cmp_si (otp->expr->value.integer, -4) != 0)
> + {
> + gfc_error ("property must be a conforming device number "
> + "at %C");
Instead of magic numbers, shouldn't this use 'include/gomp-constants.h':
/* We have a compatibility issue. OpenMP 5.2 introduced
omp_initial_device with value of -1 which clashes with our
GOMP_DEVICE_ICV, so we need to remap user supplied device
ids, -1 (aka omp_initial_device) to GOMP_DEVICE_HOST_FALLBACK,
and -2 (one of many non-conforming device numbers, but with
OMP_TARGET_OFFLOAD=mandatory needs to be treated a
omp_invalid_device) to -3 (so that for dev_num >= -2U we can
subtract 1). -4 is then what we use for omp_invalid_device,
which unlike the other non-conforming device numbers results
in fatal error regardless of OMP_TARGET_OFFLOAD. */
#define GOMP_DEVICE_ICV -1
#define GOMP_DEVICE_HOST_FALLBACK -2
#define GOMP_DEVICE_INVALID -4
Grüße
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-04 12:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-13 14:54 [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant' Tobias Burnus
2024-02-13 15:08 ` Jakub Jelinek
2024-02-13 17:31 ` [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant') Tobias Burnus
2024-02-13 17:48 ` Jakub Jelinek
2024-02-13 20:03 ` [Patch] OpenMP: Reject non-const 'condition' trait in Fortran Tobias Burnus
2024-03-04 12:58 ` [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant') Thomas Schwinge
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).