Re: [PATCH v4 6/7] OpenMP: Fortran front-end support for dispatch + adjust_args

[Paul-Antoine Arras]

Hi Tobias,

On 13/01/2025 13:24, Tobias Burnus wrote:

Hi PA,

Paul-Antoine Arras wrote:

Hi Thomas,

Added libgomp/testsuite/libgomp.fortran/dispatch-1.f90.

I see this new test case FAIL (execution test SIGSEGV) for most (but not
all) offloading configurations, both GCN and nvptx:

     +PASS: libgomp.fortran/dispatch-1.f90   -O  (test for excess errors)

     +FAIL: libgomp.fortran/dispatch-1.f90   -O  execution test

Thanks for pointing that out! The testcase missed an OpenMP target directive. The attached patch should fix it.

…

--- libgomp/testsuite/libgomp.fortran/dispatch-1.f90
+++ libgomp/testsuite/libgomp.fortran/dispatch-1.f90
@@ -55,6 +55,7 @@ module procedures
      call c_f_pointer(d_av, fp_av, [n])
      ! Perform operations on target
+    !$omp target is_device_ptr(fp_bv, fp_av)
      do i = 1, n
        fp_bv(i) = fp_av(i) * i
      end do

I think the patch is okay in the sense that it works;
still, I think you should consider the following.

Using 'is_device_ptr' for for an argument that
is not a type(c_ptr) is deprecated since OpenMP 5.1 and
removed from the specification since OpenMP 6.0.

Thus, it would be a bit cleaner (and might avoid future
-Wdeprecated warnings) using

   has_device_addr(fp_bv, fp_av) instead. (5.1 semantic states that this replacement happens automatically when the is_device_ptr argument is not a C_PTR.) Albeit it feels a bit cleaner to move the device pointer handling to the device side, i.e. implicit none integer :: res, n, i type(c_ptr) :: d_bv type(c_ptr) :: d_av !$omp target is_device_ptr(d_bv, d_av) block real(8), pointer :: fp_bv(:), fp_av(:) ! Fortran pointers for array access ! Associate C pointers with Fortran pointers call c_f_pointer(d_bv, fp_bv, [n]) call c_f_pointer(d_av, fp_av, [n]) ! Perform operations on target do i = 1, n fp_bv(i) = fp_av(i) * i end do end block However, as all variants work in practice, I don't feel strong about it, with a small preference of a variant that does not use deprecated features. (Both dispatch and the has_device_addr clause/the deprecation are new with OpenMP 5.1) Tobias

Here is an updated patch following your suggestion.

Thanks,
--
PA
commit c76b5ebf73201074cdf07842097a297e7ae4e9c5
Author: Paul-Antoine Arras <par...@baylibre.com>
Date:   Mon Jan 13 12:57:15 2025 +0100

    Add missing target directive in OpenMP dispatch Fortran runtime test
    
    Without the target directive, the test would run on the host but still try to
    use device pointers, which causes a segfault.
    
    libgomp/ChangeLog:
    
            * testsuite/libgomp.fortran/dispatch-1.f90: Add missing target
            directive.

diff --git libgomp/testsuite/libgomp.fortran/dispatch-1.f90 libgomp/testsuite/libgomp.fortran/dispatch-1.f90
index 7b2f03f9d68..f56477e4972 100644
--- libgomp/testsuite/libgomp.fortran/dispatch-1.f90
+++ libgomp/testsuite/libgomp.fortran/dispatch-1.f90
@@ -48,16 +48,21 @@ module procedures
     integer :: res, n, i
     type(c_ptr) :: d_bv
     type(c_ptr) :: d_av
-    real(8), pointer :: fp_bv(:), fp_av(:)  ! Fortran pointers for array access
 
-    ! Associate C pointers with Fortran pointers
-    call c_f_pointer(d_bv, fp_bv, [n])
-    call c_f_pointer(d_av, fp_av, [n])
+    !$omp target is_device_ptr(d_bv, d_av)
+    block
+      real(8), pointer :: fp_bv(:), fp_av(:)  ! Fortran pointers for array access
+
+      ! Associate C pointers with Fortran pointers
+      call c_f_pointer(d_bv, fp_bv, [n])
+      call c_f_pointer(d_av, fp_av, [n])
+
+      ! Perform operations on target
+      do i = 1, n
+        fp_bv(i) = fp_av(i) * i
+      end do
+    end block
 
-    ! Perform operations on target
-    do i = 1, n
-      fp_bv(i) = fp_av(i) * i
-    end do
     res = -2
   end function bar