[flang-commits] [flang] 9b16edb - [Flang][OpenMP] Fix Common Blocks use in update to/from and target maps causing compiler errors (#187221)
via flang-commits
flang-commits at lists.llvm.org
Tue Apr 7 14:19:43 PDT 2026
Author: agozillon
Date: 2026-04-07T23:19:39+02:00
New Revision: 9b16edb9ec068cca09dc01672c987aa8d3c7bdd4
URL: https://github.com/llvm/llvm-project/commit/9b16edb9ec068cca09dc01672c987aa8d3c7bdd4
DIFF: https://github.com/llvm/llvm-project/commit/9b16edb9ec068cca09dc01672c987aa8d3c7bdd4.diff
LOG: [Flang][OpenMP] Fix Common Blocks use in update to/from and target maps causing compiler errors (#187221)
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
verification 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.
Added:
flang/test/Lower/OpenMP/common-block-target-update.f90
Modified:
flang/lib/Lower/OpenMP/OpenMP.cpp
flang/lib/Semantics/resolve-directives.cpp
Removed:
################################################################################
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index bc215064bbee2..33de565eda275 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 4553c90d88f7f..9ee402b18d9bd 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -947,6 +947,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
diff erent from the parser::OmpFromClause case, as this to applies
+ // to both declare target to, and update to, so slightly
diff erent 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
diff erently 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