Re: [PATCH v2] openmp, fortran: Add Fortran support for indirect clause on the declare target directive
Hi Kwok, Kwok Cheung Yeung wrote: Oops. I thought exactly the same thing yesterday, but forgot to add the changes to my commit! Here is the updated version. I regard(ed) this change as obvious - hence, I missed to reply. But for completeness: LGTM. I think it would be useful to commit this now with an xfail for the one failing testcase that depends on the review-pending libgomp patch. I mean something like: --- a/libgomp/testsuite/libgomp.fortran/declare-target-indirect-2.f90 +++ b/libgomp/testsuite/libgomp.fortran/declare-target-indirect-2.f90 @@ -1,2 +1,3 @@ ! { dg-do run } +! { dg-xfail-run-if "Requires libgomp bug fix pending review" { offload_device } } Thanks, Tobias On 06/02/2024 9:03 am, Tobias Burnus wrote: LGTM. I just wonder whether there should be a value test and not just a does-not-crash-when-called test for the latter testcase, i.e. +++ b/libgomp/testsuite/libgomp.fortran/declare-target-indirect-3.f90 @@ -0,0 +1,25 @@ +! { dg-do run } + +! Check that indirect calls work on procedures passed in via a dummy argument + +module m +contains + subroutine bar + !$omp declare target enter(bar) indirect e.g. "integer function bar()" ... " bar = 42" + end subroutine + + subroutine foo(f) + procedure(bar) :: f + + !$omp target + call f And then: if (f() /= 42) stop 1 + !$omp end target + end subroutine +end module Thanks, Tobias
Re: [PATCH v2] openmp, fortran: Add Fortran support for indirect clause on the declare target directive
Oops. I thought exactly the same thing yesterday, but forgot to add the changes to my commit! Here is the updated version. Kwok On 06/02/2024 9:03 am, Tobias Burnus wrote: LGTM. I just wonder whether there should be a value test and not just a does-not-crash-when-called test for the latter testcase, i.e. +++ b/libgomp/testsuite/libgomp.fortran/declare-target-indirect-3.f90 @@ -0,0 +1,25 @@ +! { dg-do run } + +! Check that indirect calls work on procedures passed in via a dummy argument + +module m +contains + subroutine bar +!$omp declare target enter(bar) indirect e.g. "integer function bar()" ... " bar = 42" + end subroutine + + subroutine foo(f) +procedure(bar) :: f + +!$omp target + call f And then: if (f() /= 42) stop 1 +!$omp end target + end subroutine +end module Thanks, Tobias From 83b734aa63aa63ea5bb438bb59ee09b00869e0fd Mon Sep 17 00:00:00 2001 From: Kwok Cheung Yeung Date: Mon, 5 Feb 2024 20:31:49 + Subject: [PATCH] openmp, fortran: Add Fortran support for indirect clause on the declare target directive 2024-02-05 Kwok Cheung Yeung gcc/fortran/ * dump-parse-tree.cc (show_attr): Handle omp_declare_target_indirect attribute. * f95-lang.cc (gfc_gnu_attributes): Add entry for 'omp declare target indirect'. * gfortran.h (symbol_attribute): Add omp_declare_target_indirect field. (struct gfc_omp_clauses): Add indirect field. * openmp.cc (omp_mask2): Add OMP_CLAUSE_INDIRECT. (gfc_match_omp_clauses): Match indirect clause. (OMP_DECLARE_TARGET_CLAUSES): Add OMP_CLAUSE_INDIRECT. (gfc_match_omp_declare_target): Check omp_device_type and apply omp_declare_target_indirect attribute to symbol if indirect clause active. Show warning if there are only device_type and/or indirect clauses on the directive. * trans-decl.cc (add_attributes_to_decl): Add 'omp declare target indirect' attribute if symbol has indirect attribute set. gcc/testsuite/ * gfortran.dg/gomp/declare-target-4.f90 (f1): Update expected warning. * gfortran.dg/gomp/declare-target-indirect-1.f90: New. * gfortran.dg/gomp/declare-target-indirect-2.f90: New. libgomp/ * testsuite/libgomp.fortran/declare-target-indirect-1.f90: New. * testsuite/libgomp.fortran/declare-target-indirect-2.f90: New. * testsuite/libgomp.fortran/declare-target-indirect-3.f90: New. --- gcc/fortran/dump-parse-tree.cc| 2 + gcc/fortran/f95-lang.cc | 2 + gcc/fortran/gfortran.h| 3 +- gcc/fortran/openmp.cc | 50 ++- gcc/fortran/trans-decl.cc | 4 ++ .../gfortran.dg/gomp/declare-target-4.f90 | 2 +- .../gomp/declare-target-indirect-1.f90| 62 +++ .../gomp/declare-target-indirect-2.f90| 25 .../declare-target-indirect-1.f90 | 39 .../declare-target-indirect-2.f90 | 53 .../declare-target-indirect-3.f90 | 35 +++ 11 files changed, 272 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/gomp/declare-target-indirect-1.f90 create mode 100644 gcc/testsuite/gfortran.dg/gomp/declare-target-indirect-2.f90 create mode 100644 libgomp/testsuite/libgomp.fortran/declare-target-indirect-1.f90 create mode 100644 libgomp/testsuite/libgomp.fortran/declare-target-indirect-2.f90 create mode 100644 libgomp/testsuite/libgomp.fortran/declare-target-indirect-3.f90 diff --git a/gcc/fortran/dump-parse-tree.cc b/gcc/fortran/dump-parse-tree.cc index 1563b810b98..7b154eb3ca7 100644 --- a/gcc/fortran/dump-parse-tree.cc +++ b/gcc/fortran/dump-parse-tree.cc @@ -914,6 +914,8 @@ show_attr (symbol_attribute *attr, const char * module) fputs (" OMP-DECLARE-TARGET", dumpfile); if (attr->omp_declare_target_link) fputs (" OMP-DECLARE-TARGET-LINK", dumpfile); + if (attr->omp_declare_target_indirect) +fputs (" OMP-DECLARE-TARGET-INDIRECT", dumpfile); if (attr->elemental) fputs (" ELEMENTAL", dumpfile); if (attr->pure) diff --git a/gcc/fortran/f95-lang.cc b/gcc/fortran/f95-lang.cc index 358cb17fce2..67fda27aa3e 100644 --- a/gcc/fortran/f95-lang.cc +++ b/gcc/fortran/f95-lang.cc @@ -96,6 +96,8 @@ static const attribute_spec gfc_gnu_attributes[] = gfc_handle_omp_declare_target_attribute, NULL }, { "omp declare target link", 0, 0, true, false, false, false, gfc_handle_omp_declare_target_attribute, NULL }, + { "omp declare target indirect", 0, 0, true, false, false, false, +gfc_handle_omp_declare_target_attribute, NULL }, { "oacc function", 0, -1, true, false, false, false, gfc_handle_omp_declare_target_attribute, NULL }, }; diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index fd73e4ce431..fd843a3241d 100644 --- a/gcc/fortran/gfortran.h
Re: [PATCH v2] openmp, fortran: Add Fortran support for indirect clause on the declare target directive
Kwok Cheung Yeung wrote: As previously discussed, this version of the patch adds code to emit a warning when a directive like this: !$omp declare target indirect(.true.) is encountered (i.e. a target directive containing at least one clause, but no to/enter clause, which appears to violate the OpenMP standard). A test is also added to gfortran.dg/gomp/declare-target-indirect-1.f90 to test for this. Thanks. And indeed, the 5.1 spec requires under "Restrictions to the declare target directive are as follows:" "If the directive has a clause, it must contain at least one 'to' clause or at least one 'link' clause.". [5.2 replaced 'to' by its alias 'enter' and the 6.0 preview added 'local' to the list.] I have also added a declare-target-indirect-3.f90 test to libgomp to check that procedures passed via a dummy argument work properly when used in an indirect call. Okay for mainline? LGTM. I just wonder whether there should be a value test and not just a does-not-crash-when-called test for the latter testcase, i.e. +++ b/libgomp/testsuite/libgomp.fortran/declare-target-indirect-3.f90 @@ -0,0 +1,25 @@ +! { dg-do run } + +! Check that indirect calls work on procedures passed in via a dummy argument + +module m +contains + subroutine bar +!$omp declare target enter(bar) indirect e.g. "integer function bar()" ... " bar = 42" + end subroutine + + subroutine foo(f) +procedure(bar) :: f + +!$omp target + call f And then: if (f() /= 42) stop 1 +!$omp end target + end subroutine +end module Thanks, Tobias
[PATCH v2] openmp, fortran: Add Fortran support for indirect clause on the declare target directive
Hi As previously discussed, this version of the patch adds code to emit a warning when a directive like this: !$omp declare target indirect(.true.) is encountered (i.e. a target directive containing at least one clause, but no to/enter clause, which appears to violate the OpenMP standard). A test is also added to gfortran.dg/gomp/declare-target-indirect-1.f90 to test for this. I have also added a declare-target-indirect-3.f90 test to libgomp to check that procedures passed via a dummy argument work properly when used in an indirect call. Okay for mainline? Thanks KwokFrom f6662a7bc76d400fecb5013ad6d6ab3b00b8a6e7 Mon Sep 17 00:00:00 2001 From: Kwok Cheung Yeung Date: Mon, 5 Feb 2024 20:31:49 + Subject: [PATCH] openmp, fortran: Add Fortran support for indirect clause on the declare target directive 2024-02-05 Kwok Cheung Yeung gcc/fortran/ * dump-parse-tree.cc (show_attr): Handle omp_declare_target_indirect attribute. * f95-lang.cc (gfc_gnu_attributes): Add entry for 'omp declare target indirect'. * gfortran.h (symbol_attribute): Add omp_declare_target_indirect field. (struct gfc_omp_clauses): Add indirect field. * openmp.cc (omp_mask2): Add OMP_CLAUSE_INDIRECT. (gfc_match_omp_clauses): Match indirect clause. (OMP_DECLARE_TARGET_CLAUSES): Add OMP_CLAUSE_INDIRECT. (gfc_match_omp_declare_target): Check omp_device_type and apply omp_declare_target_indirect attribute to symbol if indirect clause active. Show warning if there are only device_type and/or indirect clauses on the directive. * trans-decl.cc (add_attributes_to_decl): Add 'omp declare target indirect' attribute if symbol has indirect attribute set. gcc/testsuite/ * gfortran.dg/gomp/declare-target-4.f90 (f1): Update expected warning. * gfortran.dg/gomp/declare-target-indirect-1.f90: New. * gfortran.dg/gomp/declare-target-indirect-2.f90: New. libgomp/ * testsuite/libgomp.fortran/declare-target-indirect-1.f90: New. * testsuite/libgomp.fortran/declare-target-indirect-2.f90: New. * testsuite/libgomp.fortran/declare-target-indirect-3.f90: New. --- gcc/fortran/dump-parse-tree.cc| 2 + gcc/fortran/f95-lang.cc | 2 + gcc/fortran/gfortran.h| 3 +- gcc/fortran/openmp.cc | 50 ++- gcc/fortran/trans-decl.cc | 4 ++ .../gfortran.dg/gomp/declare-target-4.f90 | 2 +- .../gomp/declare-target-indirect-1.f90| 62 +++ .../gomp/declare-target-indirect-2.f90| 25 .../declare-target-indirect-1.f90 | 39 .../declare-target-indirect-2.f90 | 53 .../declare-target-indirect-3.f90 | 25 11 files changed, 262 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/gomp/declare-target-indirect-1.f90 create mode 100644 gcc/testsuite/gfortran.dg/gomp/declare-target-indirect-2.f90 create mode 100644 libgomp/testsuite/libgomp.fortran/declare-target-indirect-1.f90 create mode 100644 libgomp/testsuite/libgomp.fortran/declare-target-indirect-2.f90 create mode 100644 libgomp/testsuite/libgomp.fortran/declare-target-indirect-3.f90 diff --git a/gcc/fortran/dump-parse-tree.cc b/gcc/fortran/dump-parse-tree.cc index 1563b810b98..7b154eb3ca7 100644 --- a/gcc/fortran/dump-parse-tree.cc +++ b/gcc/fortran/dump-parse-tree.cc @@ -914,6 +914,8 @@ show_attr (symbol_attribute *attr, const char * module) fputs (" OMP-DECLARE-TARGET", dumpfile); if (attr->omp_declare_target_link) fputs (" OMP-DECLARE-TARGET-LINK", dumpfile); + if (attr->omp_declare_target_indirect) +fputs (" OMP-DECLARE-TARGET-INDIRECT", dumpfile); if (attr->elemental) fputs (" ELEMENTAL", dumpfile); if (attr->pure) diff --git a/gcc/fortran/f95-lang.cc b/gcc/fortran/f95-lang.cc index 358cb17fce2..67fda27aa3e 100644 --- a/gcc/fortran/f95-lang.cc +++ b/gcc/fortran/f95-lang.cc @@ -96,6 +96,8 @@ static const attribute_spec gfc_gnu_attributes[] = gfc_handle_omp_declare_target_attribute, NULL }, { "omp declare target link", 0, 0, true, false, false, false, gfc_handle_omp_declare_target_attribute, NULL }, + { "omp declare target indirect", 0, 0, true, false, false, false, +gfc_handle_omp_declare_target_attribute, NULL }, { "oacc function", 0, -1, true, false, false, false, gfc_handle_omp_declare_target_attribute, NULL }, }; diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index fd73e4ce431..fd843a3241d 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -999,6 +999,7 @@ typedef struct /* Mentioned in OMP DECLARE TARGET. */ unsigned omp_declare_target:1; unsigned omp_declare_target_link:1; + unsigned omp_declare_target_indirect:1; ENUM_BITFIELD (gfc_omp_