[flang-commits] [flang] [llvm] [flang][OpenMP] Fix mapping of constant arrays. (PR #176763)
Abid Qadeer via flang-commits
flang-commits at lists.llvm.org
Tue Jan 20 05:58:50 PST 2026
https://github.com/abidh updated https://github.com/llvm/llvm-project/pull/176763
>From ff7e5013ba0f238932af99b30d0aef9b660bac80 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Mon, 19 Jan 2026 14:51:56 +0000
Subject: [PATCH 1/2] [flang][OpenMP] Fix mapping of constant arrays.
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 s_compute_chemistry_reaction_flux(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 s_compute_chemistry_reaction_flux
end module fir_lowering_check
program fir_mvp_prog
use fir_lowering_check
implicit none
integer :: hold
hold = 1
call s_compute_chemistry_reaction_flux(hold)
print *, "Finished"
end program fir_mvp_prog`
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".
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 11 +++-
.../Lower/OpenMP/target-parameter-array.f90 | 60 +++++++++++++++++++
2 files changed, 69 insertions(+), 2 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/target-parameter-array.f90
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
>From b0e3a98ebaf4dbf5d9809c91419a5a90d70603bb Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 20 Jan 2026 13:24:01 +0000
Subject: [PATCH 2/2] Handle review comments.
1. Make sure that character string gets mapped if a dynamic substring is accessed in the target region.
2. Check the behavior added in 1.
3. Add an offload testcase.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 71 +++++++++-
.../Lower/OpenMP/target-parameter-array.f90 | 31 ++++-
.../fortran/target-parameter-array.f90 | 131 ++++++++++++++++++
3 files changed, 231 insertions(+), 2 deletions(-)
create mode 100644 offload/test/offloading/fortran/target-parameter-array.f90
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index dddb02adffa36..8e0e9f411fa67 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -18,6 +18,7 @@
#include "Decomposer.h"
#include "Utils.h"
#include "flang/Common/idioms.h"
+#include "flang/Evaluate/tools.h"
#include "flang/Evaluate/type.h"
#include "flang/Lower/Bridge.h"
#include "flang/Lower/ConvertCall.h"
@@ -2573,6 +2574,66 @@ static bool isDuplicateMappedSymbol(
return checkSymbol(sym.GetUltimate());
}
+// Visitor to collect symbols that have dynamic substring accesses
+struct DynamicSubstringVisitor {
+ llvm::SmallPtrSet<const semantics::Symbol *, 8> symbolsWithDynamicSubstring;
+ semantics::SemanticsContext &semaCtx;
+
+ explicit DynamicSubstringVisitor(semantics::SemanticsContext &ctx)
+ : semaCtx(ctx) {}
+
+ template <typename T>
+ bool Pre(const T &) {
+ return true;
+ }
+ template <typename T>
+ void Post(const T &) {}
+
+ // Check each expression for substring access
+ void Post(const parser::Expr &expr) {
+ if (const auto *typedExpr = semantics::GetExpr(semaCtx, expr)) {
+ // Try to extract a substring from this expression
+ if (auto substring = Fortran::evaluate::ExtractSubstring(*typedExpr)) {
+ // Check if the substring has non-constant (dynamic) indices
+ bool hasDynamicIndex = false;
+
+ // Check if explicit lower bound exists and is non-constant
+ if (const auto *lowerExpr = substring->GetLower()) {
+ if (!Fortran::evaluate::ToInt64(*lowerExpr))
+ hasDynamicIndex = true;
+ }
+
+ // Check if explicit upper bound exists and is non-constant
+ if (const auto *upperExpr = substring->GetUpper()) {
+ if (!Fortran::evaluate::ToInt64(*upperExpr))
+ hasDynamicIndex = true;
+ }
+
+ // If we have dynamic indices, extract the base symbol
+ if (hasDynamicIndex) {
+ if (auto dataRef =
+ Fortran::evaluate::ExtractSubstringBase(*substring)) {
+ if (const auto *symRef =
+ std::get_if<Fortran::evaluate::SymbolRef>(&dataRef->u)) {
+ symbolsWithDynamicSubstring.insert(&symRef->get());
+ }
+ }
+ }
+ }
+ }
+ }
+};
+
+// Collect symbols that have dynamic substring accesses in the target region
+static void collectSymbolsWithDynamicSubstring(
+ semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
+ llvm::SmallPtrSet<const semantics::Symbol *, 8>
+ &symbolsWithDynamicSubstring) {
+ DynamicSubstringVisitor visitor(semaCtx);
+ eval.visit([&](const auto &node) { parser::Walk(node, visitor); });
+ symbolsWithDynamicSubstring = visitor.symbolsWithDynamicSubstring;
+}
+
static mlir::omp::TargetOp
genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
lower::StatementContext &stmtCtx,
@@ -2635,6 +2696,11 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
/*isTargetPrivitization=*/true);
dsp.processStep1(&clauseOps);
+ // Collect symbols that have dynamic substring accesses
+ llvm::SmallPtrSet<const semantics::Symbol *, 8> symbolsWithDynamicSubstring;
+ collectSymbolsWithDynamicSubstring(semaCtx, eval,
+ symbolsWithDynamicSubstring);
+
// 5.8.1 Implicit Data-Mapping Attribute Rules
// The following code follows the implicit data-mapping rules to map all the
// symbols used inside the region that do not have explicit data-environment
@@ -2665,7 +2731,10 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
// 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)
+ // Also, character scalar parameters must be mapped if they have dynamic
+ // substring access.
+ if (semantics::IsNamedConstant(sym) && sym.Rank() == 0 &&
+ !symbolsWithDynamicSubstring.contains(&sym.GetUltimate()))
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
index f494a6d479370..39218ddf5ef58 100644
--- a/flang/test/Lower/OpenMP/target-parameter-array.f90
+++ b/flang/test/Lower/OpenMP/target-parameter-array.f90
@@ -13,7 +13,7 @@ module param_array_module
contains
-! Test 1: Parameter array with dynamic index in target region with teams distribute
+! 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
@@ -57,4 +57,33 @@ subroutine test_scalar_param()
end subroutine test_scalar_param
+! Test character scalar parameter with dynamic substring access
+! CHECK-LABEL: func.func @_QMparam_array_modulePtest_char_substring
+subroutine test_char_substring(start_idx, end_idx)
+ integer, intent(in) :: start_idx, end_idx
+ character(len=20), parameter :: char_scalar = "constant_string_data"
+ character(len=10) :: result
+
+ ! CHECK: omp.map.info{{.*}}map_clauses(implicit, to){{.*}}{name = "char_scalar"}
+ !$omp target
+ ! Dynamic substring access - character scalar must be mapped
+ result = char_scalar(start_idx:end_idx)
+ !$omp end target
+
+end subroutine test_char_substring
+
+! Verify character scalar with constant substring is NOT mapped
+! CHECK-LABEL: func.func @_QMparam_array_modulePtest_char_const_substring
+subroutine test_char_const_substring()
+ character(len=20), parameter :: char_const = "constant_string_data"
+ character(len=5) :: result
+
+ ! CHECK-NOT: omp.map.info{{.*}}{name = "char_const"}
+ !$omp target
+ ! Constant substring access - can be inlined, no mapping needed
+ result = char_const(1:5)
+ !$omp end target
+
+end subroutine test_char_const_substring
+
end module param_array_module
diff --git a/offload/test/offloading/fortran/target-parameter-array.f90 b/offload/test/offloading/fortran/target-parameter-array.f90
new file mode 100644
index 0000000000000..b85fb067803d2
--- /dev/null
+++ b/offload/test/offloading/fortran/target-parameter-array.f90
@@ -0,0 +1,131 @@
+! Offload test for parameter (constant) arrays and character scalars accessed
+! with dynamic indices/substrings in OpenMP target regions.
+
+! REQUIRES: flang, amdgpu
+
+! RUN: %libomptarget-compile-fortran-run-and-check-generic
+
+program test_parameter_mapping
+ implicit none
+ integer, parameter :: dp = selected_real_kind(15, 307)
+ logical :: all_tests_pass
+
+ all_tests_pass = .true.
+
+ ! Test 1: Parameter array with dynamic index
+ call test_param_array_dynamic_index(all_tests_pass)
+
+ ! Test 2: Integer parameter array
+ call test_int_param_array(all_tests_pass)
+
+ ! Test 3: Character scalar with dynamic substring
+ call test_char_substring(all_tests_pass)
+
+ ! Test 4: Verify scalar parameters work (inlined)
+ call test_scalar_param(all_tests_pass)
+
+ if (all_tests_pass) then
+ print *, "PASS"
+ else
+ print *, "FAIL"
+ endif
+
+contains
+
+! Test 1: Parameter array with dynamic index in target region
+subroutine test_param_array_dynamic_index(test_pass)
+ logical, intent(inout) :: test_pass
+ real(dp), parameter :: const_array(3) = [1.0_dp, 2.0_dp, 3.0_dp]
+ integer :: idx
+ real(dp) :: result
+ real(dp), parameter :: expected = 2.0_dp
+ real(dp), parameter :: tolerance = 1.0e-10_dp
+
+ idx = 2
+ result = 0.0_dp
+
+ !$omp target map(tofrom:result) map(to:idx)
+ ! Access parameter array with dynamic index
+ result = const_array(idx)
+ !$omp end target
+
+ if (abs(result - expected) > tolerance) then
+ print *, "Test 1 FAILED: expected", expected, "got", result
+ test_pass = .false.
+ endif
+end subroutine test_param_array_dynamic_index
+
+! Test 2: Integer parameter array with different indices
+subroutine test_int_param_array(test_pass)
+ logical, intent(inout) :: test_pass
+ integer, parameter :: int_array(4) = [10, 20, 30, 40]
+ integer :: idx1, idx2
+ integer :: result1, result2
+
+ idx1 = 1
+ idx2 = 4
+ result1 = 0
+ result2 = 0
+
+ !$omp target map(tofrom:result1, result2) map(to:idx1, idx2)
+ ! Access parameter array with different dynamic indices
+ result1 = int_array(idx1)
+ result2 = int_array(idx2)
+ !$omp end target
+
+ if (result1 /= 10 .or. result2 /= 40) then
+ print *, "Test 2 FAILED: expected 10, 40 got", result1, result2
+ test_pass = .false.
+ endif
+end subroutine test_int_param_array
+
+! Test 3: Character scalar parameter with dynamic substring access
+subroutine test_char_substring(test_pass)
+ logical, intent(inout) :: test_pass
+ character(len=20), parameter :: char_scalar = "constant_string_data"
+ integer :: start_idx, end_idx
+ character(len=8) :: result
+ character(len=8), parameter :: expected = "string_d"
+
+ start_idx = 10
+ end_idx = 17
+ result = ""
+
+ !$omp target map(tofrom:result) map(to:start_idx, end_idx)
+ ! Dynamic substring access - character scalar must be mapped
+ result = char_scalar(start_idx:end_idx)
+ !$omp end target
+
+ if (result /= expected) then
+ print *, "Test 3 FAILED: expected '", expected, "' got '", result, "'"
+ test_pass = .false.
+ endif
+end subroutine test_char_substring
+
+! Test 4: Scalar parameter (can be inlined, no mapping needed)
+subroutine test_scalar_param(test_pass)
+ logical, intent(inout) :: test_pass
+ integer, parameter :: scalar_const = 42
+ real(dp), parameter :: real_const = 3.14159_dp
+ integer :: int_result
+ real(dp) :: real_result
+ real(dp), parameter :: tolerance = 1.0e-5_dp
+
+ int_result = 0
+ real_result = 0.0_dp
+
+ !$omp target map(tofrom:int_result, real_result)
+ ! Scalar parameters should be inlined (no mapping needed)
+ int_result = scalar_const
+ real_result = real_const
+ !$omp end target
+
+ if (int_result /= 42 .or. abs(real_result - real_const) > tolerance) then
+ print *, "Test 4 FAILED: expected 42, 3.14159 got", int_result, real_result
+ test_pass = .false.
+ endif
+end subroutine test_scalar_param
+
+end program test_parameter_mapping
+
+! CHECK: PASS
More information about the flang-commits
mailing list