Re: [PATCH v2] openmp, fortran: Add Fortran support for indirect clause on the declare target directive

2024-02-12 Thread Tobias Burnus 

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

2024-02-06 Thread Kwok Cheung Yeung 
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
---

Re: [PATCH v2] openmp, fortran: Add Fortran support for indirect clause on the declare target directive

2024-02-06 Thread Tobias Burnus 

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