[flang-commits] [flang] [Flang][OpenMP] Fix Common Blocks use in update to/from and target maps causing compiler errors (PR #187221)
via flang-commits
flang-commits at lists.llvm.org
Wed Mar 18 02:24:44 PDT 2026
https://github.com/agozillon updated https://github.com/llvm/llvm-project/pull/187221
>From f0dec34c478acb2da5e84f1f0d486979a2b75bb1 Mon Sep 17 00:00:00 2001
From: agozillon <Andrew.Gozillon at amd.com>
Date: Wed, 18 Mar 2026 04:07:35 -0500
Subject: [PATCH] [Flang][OpenMP] Fix Common Blocks use in update to/from and
target maps causing compiler errors
This patch attempts to fix a compiler ICE when common blocks are used in target update to/from,
it seems to stem from the fact that we do not resolve the symbols in the relevant clauses, so
when we later process the maps we don't have the right symbol that references the common block
that was setup and bound by the fortran lowering. Resolving the names seems to do the trick.
There is a second issue where when referencing a common block with an array contained in it
and utilising the array within the target region, we'll currently not accurately map over
the bounds and cause a FIR/MLIR veriifcation error. The fix for this is to simply move the
common block member re-binding/re-materialization for the target region to before the bounds
data re-materialization we do during target region generation.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 34 ++++++++++-----
flang/lib/Semantics/resolve-directives.cpp | 41 +++++++++++++++++++
.../OpenMP/common-block-target-update.f90 | 30 ++++++++++++++
3 files changed, 95 insertions(+), 10 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/common-block-target-update.f90
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index ae5f6f50bda09..1efd4ee6489e4 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -980,6 +980,20 @@ getImplicitMapTypeAndKind(fir::FirOpBuilder &firOpBuilder,
mlir::Operation *op = mod.lookupSymbol(converter.mangleName(sym));
auto declareTargetOp =
llvm::dyn_cast_if_present<mlir::omp::DeclareTargetInterface>(op);
+
+ // Double check it's not part of a common block, and that the common block
+ // isn't marked declare target.
+ if (!declareTargetOp) {
+ if (const semantics::Symbol *common =
+ semantics::FindCommonBlockContaining(sym.GetUltimate())) {
+ mlir::Operation *commonOp =
+ mod.lookupSymbol(converter.mangleName(*common));
+ declareTargetOp =
+ llvm::dyn_cast_if_present<mlir::omp::DeclareTargetInterface>(
+ commonOp);
+ }
+ }
+
if (declareTargetOp && declareTargetOp.isDeclareTarget()) {
if (declareTargetOp.getDeclareTargetCaptureClause() ==
mlir::omp::DeclareTargetCaptureClause::link &&
@@ -1414,6 +1428,16 @@ static void genBodyOfTargetOp(
if (HostEvalInfo *hostEvalInfo = getHostEvalInfoStackTop(converter))
hostEvalInfo->bindOperands(argIface.getHostEvalBlockArgs());
+ // If we map a common block using it's symbol e.g. map(tofrom: /common_block/)
+ // and accessing its members within the target region, there is a large
+ // chance we will end up with uses external to the region accessing the common
+ // resolve these, we do so by generating new common block member accesses
+ // within the region, binding them to the member symbol for the scope of the
+ // region so that subsequent code generation within the region will utilise
+ // our new member accesses we have created.
+ genIntermediateCommonBlockAccessors(
+ converter, currentLocation, argIface.getMapBlockArgs(), args.map.syms);
+
// Check if cloning the bounds introduced any dependency on the outer region.
// If so, then either clone them as well if they are MemoryEffectFree, or else
// copy them to a new temporary and add them to the map and block_argument
@@ -1440,16 +1464,6 @@ static void genBodyOfTargetOp(
// Create the insertion point after the marker.
firOpBuilder.setInsertionPointAfter(undefMarker.getDefiningOp());
- // If we map a common block using it's symbol e.g. map(tofrom: /common_block/)
- // and accessing its members within the target region, there is a large
- // chance we will end up with uses external to the region accessing the common
- // resolve these, we do so by generating new common block member accesses
- // within the region, binding them to the member symbol for the scope of the
- // region so that subsequent code generation within the region will utilise
- // our new member accesses we have created.
- genIntermediateCommonBlockAccessors(
- converter, currentLocation, argIface.getMapBlockArgs(), args.map.syms);
-
if (ConstructQueue::const_iterator next = std::next(item);
next != queue.end()) {
genOMPDispatch(converter, symTable, semaCtx, eval, currentLocation, queue,
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 96f66246ac676..95d7b501490fe 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -942,6 +942,47 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
void Post(const parser::EndLabel &endLabel) { CheckSourceLabel(endLabel.v); }
void Post(const parser::EorLabel &eorLabel) { CheckSourceLabel(eorLabel.v); }
+ void ResolveOmpObjectsForMapClause(
+ Symbol::Flag mapFlag, const parser::OmpObjectList &objList) {
+ for (const auto &ompObj : objList.v) {
+ common::visit(
+ common::visitors{
+ [&](const parser::Designator &designator) {
+ if (const auto *name{
+ parser::GetDesignatorNameIfDataRef(designator)}) {
+ if (name->symbol) {
+ name->symbol->set(mapFlag);
+ }
+ }
+ },
+ [&](const auto &name) {},
+ },
+ ompObj.u);
+
+ ResolveOmpObject(ompObj, mapFlag);
+ }
+ }
+
+ void Post(const parser::OmpFromClause &x) {
+ const auto &ompObjList{*parser::omp::GetOmpObjectList(x)};
+ ResolveOmpObjectsForMapClause(Symbol::Flag::OmpMapFrom, ompObjList);
+ }
+
+ void Post(const parser::OmpToClause &x) {
+ // This is different from the parser::OmpFromClause case, as this to applies
+ // to both declare target to, and update to, so slightly different handling
+ // is required in that we must exit early to avoid applying extra symbol
+ // flags. This is reasonable for now, but if we wish to apply this
+ // resolution to declare target to (and likely enter in another function
+ // like this) we will have to extend the handling to act differently for
+ // declare target rather than simply return.
+ if (GetContext().directive == llvm::omp::Directive::OMPD_declare_target)
+ return;
+
+ const auto &ompObjList{*parser::omp::GetOmpObjectList(x)};
+ ResolveOmpObjectsForMapClause(Symbol::Flag::OmpMapTo, ompObjList);
+ }
+
void Post(const parser::OmpMapClause &x) {
unsigned version{context_.langOptions().OpenMPVersion};
std::optional<Symbol::Flag> ompFlag;
diff --git a/flang/test/Lower/OpenMP/common-block-target-update.f90 b/flang/test/Lower/OpenMP/common-block-target-update.f90
new file mode 100644
index 0000000000000..ff328e3335633
--- /dev/null
+++ b/flang/test/Lower/OpenMP/common-block-target-update.f90
@@ -0,0 +1,30 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+program main
+ implicit none
+ common/cb_array/array
+ real(kind=8), dimension(100) :: array
+ integer i
+
+ !$omp target update to(/cb_array/)
+
+ !$omp target teams distribute parallel do map(tofrom: /cb_array/)
+ do i = 1, 100
+ array(i) = i
+ enddo
+ !$omp end target teams distribute parallel do
+
+ !$omp target update from(/cb_array/)
+end program
+
+! CHECK-LABEL: func.func @_QQmain()
+
+! CHECK: %[[CB_ADDR:.*]] = fir.address_of(@cb_array_) : !fir.ref<!fir.array<800xi8>>
+! CHECK: %[[MAP_TO:.*]] = omp.map.info var_ptr(%[[CB_ADDR]] : !fir.ref<!fir.array<800xi8>>, !fir.array<800xi8>) map_clauses(to) capture(ByRef) -> !fir.ref<!fir.array<800xi8>> {name = "cb_array"}
+! CHECK: omp.target_update map_entries(%[[MAP_TO]] : !fir.ref<!fir.array<800xi8>>)
+
+! CHECK: %[[MAP_TOFROM:.*]] = omp.map.info var_ptr(%[[CB_ADDR]] : !fir.ref<!fir.array<800xi8>>, !fir.array<800xi8>) map_clauses(tofrom) capture(ByRef) -> !fir.ref<!fir.array<800xi8>> {name = "cb_array"}
+! CHECK: omp.target {{.*}} map_entries(%[[MAP_TOFROM]] -> %{{.*}}, %{{.*}} -> %{{.*}} : !fir.ref<!fir.array<800xi8>>, !fir.ref<i32>)
+
+! CHECK: %[[MAP_FROM:.*]] = omp.map.info var_ptr(%[[CB_ADDR]] : !fir.ref<!fir.array<800xi8>>, !fir.array<800xi8>) map_clauses(from) capture(ByRef) -> !fir.ref<!fir.array<800xi8>> {name = "cb_array"}
+! CHECK: omp.target_update map_entries(%[[MAP_FROM]] : !fir.ref<!fir.array<800xi8>>)
More information about the flang-commits
mailing list