[flang-commits] [flang] [flang][OpenMP] Fix mapping of constant arrays. (PR #176763)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-openmp

Author: Abid Qadeer (abidh)

<details>
<summary>Changes</summary>

The compiler skips mapping of named constants (parameters) to OpenMP target regions under the assumption that constants don't need to be mapped. This assumption is not valid when array is accessed inside with dynamic index. The problem can be seen with the following code:

```
module fir_lowering_check
  implicit none

    integer, parameter :: dp = selected_real_kind(15, 307)
    real(dp), parameter :: arrays(2) = (/ 0.0, 0.0 /)

contains

subroutine test(hold)

        integer, intent(in) :: hold
        integer :: z
        real(dp) :: temp

        !$omp target teams distribute parallel do
            do z = 1, 2
                  temp = arrays(hold)
            end do
        !$omp end target teams distribute parallel do

    end subroutine test
end module fir_lowering_check

program main
  use fir_lowering_check

  implicit none
    integer :: hold
    hold = 1
    call test(hold)
    print *, "Finished"

end program main
```

It fails with the following error
`'hlfir.designate' op using value defined outside the region`

The fix is to allow mapping of constant arrays and map them as `to`.

---
Full diff: https://github.com/llvm/llvm-project/pull/176763.diff


2 Files Affected:

- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+9-2) 
- (added) flang/test/Lower/OpenMP/target-parameter-array.f90 (+60) 


``````````diff
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 0764693f748a5..dddb02adffa36 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -995,6 +995,11 @@ getImplicitMapTypeAndKind(fir::FirOpBuilder &firOpBuilder,
       } else {
         mapFlag |= mlir::omp::ClauseMapFlags::to;
       }
+    } else if (semantics::IsNamedConstant(sym)) {
+      // Parameter constants should be mapped as read-only (to) since they
+      // cannot be modified. Mapping them as tofrom would cause a crash when
+      // trying to write back to read-only memory.
+      mapFlag |= mlir::omp::ClauseMapFlags::to;
     } else if (!fir::isa_builtin_cptr_type(varType)) {
       mapFlag |= mlir::omp::ClauseMapFlags::to;
       mapFlag |= mlir::omp::ClauseMapFlags::from;
@@ -2657,8 +2662,10 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
     if (!converter.getSymbolAddress(sym))
       return;
 
-    // Skip parameters/constants as they do not need to be mapped.
-    if (semantics::IsNamedConstant(sym))
+    // Skip scalar parameters/constants as they do not need to be mapped.
+    // However, parameter arrays must be mapped as they may be accessed with
+    // dynamic indices on the device (e.g., const_array(runtime_index)).
+    if (semantics::IsNamedConstant(sym) && sym.Rank() == 0)
       return;
 
     if (!isDuplicateMappedSymbol(sym, dsp.getAllSymbolsToPrivatize(),
diff --git a/flang/test/Lower/OpenMP/target-parameter-array.f90 b/flang/test/Lower/OpenMP/target-parameter-array.f90
new file mode 100644
index 0000000000000..f494a6d479370
--- /dev/null
+++ b/flang/test/Lower/OpenMP/target-parameter-array.f90
@@ -0,0 +1,60 @@
+! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+! Test that parameter (constant) arrays can be mapped to OpenMP target regions
+! and are mapped as read-only when accessed with dynamic indices.
+
+module param_array_module
+  implicit none
+  integer, parameter :: dp = selected_real_kind(15, 307)
+
+  ! Parameter arrays that should be mapped to device
+  real(dp), parameter :: const_array(3) = [1.0_dp, 2.0_dp, 3.0_dp]
+  integer, parameter :: int_array(4) = [10, 20, 30, 40]
+
+contains
+
+! Test 1: Parameter array with dynamic index in target region with teams distribute
+! CHECK-LABEL: func.func @_QMparam_array_modulePtest_param_array_target
+subroutine test_param_array_target(idx)
+  integer, intent(in) :: idx
+  integer :: i
+  real(dp) :: result
+
+  ! CHECK: omp.map.info{{.*}}map_clauses(implicit, to){{.*}}{name = "const_array"}
+  !$omp target teams distribute parallel do
+  do i = 1, 3
+    ! Access parameter array with dynamic index
+    result = const_array(idx)
+  end do
+  !$omp end target teams distribute parallel do
+
+end subroutine test_param_array_target
+
+! Integer parameter array in simple target region
+! CHECK-LABEL: func.func @_QMparam_array_modulePtest_int_param_array
+subroutine test_int_param_array(idx)
+  integer, intent(in) :: idx
+  integer :: result
+
+  ! CHECK: omp.map.info{{.*}}map_clauses(implicit, to){{.*}}{name = "int_array"}
+  !$omp target
+    ! Access parameter array with dynamic index
+    result = int_array(idx)
+  !$omp end target
+
+end subroutine test_int_param_array
+
+! Verify scalar parameters are NOT mapped (can be inlined)
+! CHECK-LABEL: func.func @_QMparam_array_modulePtest_scalar_param
+subroutine test_scalar_param()
+  integer, parameter :: scalar_const = 42
+  integer :: result
+
+  ! CHECK-NOT: omp.map.info{{.*}}{name = "scalar_const"}
+  !$omp target
+    result = scalar_const
+  !$omp end target
+
+end subroutine test_scalar_param
+
+end module param_array_module

``````````

</details>


https://github.com/llvm/llvm-project/pull/176763