[flang-commits] [flang] [flang][OpenMP] Mark declare target on use-associated module variables (PR #214596)

Thu Aug 6 16:48:33 PDT 2026

llvmorg-github-actions[bot] wrote:



@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Spencer Bryngelson (sbryngelson)

<details>
<summary>Changes</summary>

Fixes #214586.

A module variable marked `!$omp declare target` keeps its `OmpDeclareTarget` flag when it is use-associated into another translation unit, and the `.mod` file records the directive. But no declare target directive is parsed in that unit, so neither `markDeclareTarget` call site (`symbolAndClause` and `deferredDeclareTargets`) reaches it, and the global is emitted with no `omp.declare_target`.

`HostOpFiltering` then treats it as an ordinary host global and gives it internal linkage for the device:

```llvm
; consuming TU, --offload-device-only -O0
@_QMmod_aEnsz = internal addrspace(1) global i32 undef
```

so a `declare target` routine in that TU reads an uninitialized private copy instead of the definition, and `target update to(...)` has no effect on it. At -O2 the load from an `internal undef` global folds away, the dependent store is removed, and the `intent(out)` dummy becomes `readnone`. No diagnostic; the program computes wrong results. The issue has a three-file reproducer and a same-TU control that passes.

`genOpenMPSymbolProperties` already tests the flag and is reached for the use-associated variable, but its declare target branch calls `genDeclareTargetIntGlobal`, which starts with `if (!var.isGlobal())` and only materializes a `GlobalOp` for main program SAVE variables, so a module variable falls through unmarked.

This marks the declaration from the capture clause and device type recorded on the defining symbol.

Only globals **without an initializer body** are marked. A global defined in this unit is marked later by `markDeclareTarget` with the clauses as written; marking it here first would discard them, because `markDeclareTarget` leaves an already marked operation alone. That is not hypothetical -- an earlier version of this patch gated on `isDeclareTarget()` instead and regressed `Lower/OpenMP/declare-target-automap.f90`, because the main program global is not yet marked at this point.

`automap` is a modifier rather than a clause, so it is not in the symbol's clause set and cannot be recovered for a use-associated symbol; `false` is passed. That is a gap, though the current behaviour for such a variable is a silent wrong answer.

### Testing

- New `flang/test/Lower/OpenMP/declare-target-modfile.f90`, modelled on the existing `groupprivate-modfile.f90` cross-TU pattern. Verified it fails without the change and passes with it.
- `mlir/test/Dialect/OpenMP`, `mlir/test/Target/LLVMIR`, `flang/test/Lower/OpenMP`, `flang/test/Integration/OpenMP`, `flang/test/Transforms`, `flang/test/Fir`, `flang/test/Semantics/OpenMP`: 1921 tests, no regressions.
- The issue's reproducer runs correctly on gfx90a (MI250X), returning 42 instead of the host sentinel.

Developed and tested at 785fd14fa94e; the touched code is identical in main, where the patch applies cleanly.

cc @skatrak


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


2 Files Affected:

- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+65-1) 
- (added) flang/test/Lower/OpenMP/declare-target-modfile.f90 (+31) 


``````````diff

diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 3876799b3a081..bf22e998acd2f 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -6906,6 +6906,68 @@ void Fortran::lower::genOpenMPDeclarativeConstruct(
   genNestedEvaluations(converter, eval);
 }
 
+/// A declare target module variable keeps its \c OmpDeclareTarget flag when it
+/// is use-associated into another translation unit, but no declare target
+/// directive is parsed there, so neither \c markDeclareTarget call site reaches
+/// it. Attach the attribute to the declaration from what the defining symbol
+/// recorded.
+///
+/// Without this the declaration is indistinguishable from an ordinary host
+/// global and is given internal linkage for the device, replacing the
+/// definition with an undefined local copy and silently producing wrong
+/// results.
+static void
+markUseAssociatedDeclareTarget(lower::AbstractConverter &converter,
+                               const lower::pft::Variable &var) {
+  if (!var.isGlobal())
+    return;
+
+  const semantics::Symbol &ultimate = var.getSymbol().GetUltimate();
+  const auto *details = ultimate.detailsIf<semantics::ObjectEntityDetails>();
+  if (!details)
+    return;
+
+  const semantics::WithOmpDeclarative::OmpClauseSet &clauses =
+      details->ompDeclTarget();
+  if (!clauses.count())
+    return;
+
+  mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
+  mlir::Operation *op = mod.lookupSymbol(converter.mangleName(ultimate));
+  if (!op)
+    return;
+
+  auto declareTargetOp = llvm::dyn_cast<mlir::omp::DeclareTargetInterface>(op);
+  if (!declareTargetOp || declareTargetOp.isDeclareTarget())
+    return;
+
+  // Only mark declarations. A global defined in this unit carries an
+  // initializer body and is marked by markDeclareTarget with the clauses as
+  // written, which happens after this point; marking it here would discard
+  // them, because markDeclareTarget leaves an already marked operation alone.
+  for (mlir::Region &region : op->getRegions())
+    if (!region.empty())
+      return;
+
+  // `enter` and `to` are equivalent captures and lower identically, matching
+  // what the directive path produces for the definition.
+  mlir::omp::DeclareTargetCaptureClause captureClause =
+      clauses.test(llvm::omp::Clause::OMPC_link)
+          ? mlir::omp::DeclareTargetCaptureClause::link
+          : mlir::omp::DeclareTargetCaptureClause::to;
+
+  mlir::omp::DeclareTargetDeviceType deviceType =
+      mlir::omp::DeclareTargetDeviceType::any;
+  if (const std::optional<common::OmpDeviceType> &dt =
+          details->ompDeclTargetDeviceType())
+    deviceType = toMLIRDeclareTargetDeviceType(*dt);
+
+  // automap is a modifier rather than a clause, so it is not recorded in the
+  // clause set and cannot be recovered here for a use-associated symbol.
+  declareTargetOp.setDeclareTarget(deviceType, captureClause,
+                                   /*automap=*/false);
+}
+
 void Fortran::lower::genOpenMPSymbolProperties(
     lower::AbstractConverter &converter, const lower::pft::Variable &var) {
   assert(var.hasSymbol() && "Expecting Symbol");
@@ -6917,8 +6979,10 @@ void Fortran::lower::genOpenMPSymbolProperties(
   if (sym.test(semantics::Symbol::Flag::OmpThreadprivate))
     lower::genThreadprivateOp(converter, var);
 
-  if (sym.test(semantics::Symbol::Flag::OmpDeclareTarget))
+  if (sym.test(semantics::Symbol::Flag::OmpDeclareTarget)) {
     lower::genDeclareTargetIntGlobal(converter, var);
+    markUseAssociatedDeclareTarget(converter, var);
+  }
 }
 
 void Fortran::lower::genGroupprivateOp(lower::AbstractConverter &converter,
diff --git a/flang/test/Lower/OpenMP/declare-target-modfile.f90 b/flang/test/Lower/OpenMP/declare-target-modfile.f90
new file mode 100644
index 0000000000000..43eed5ef55c69
--- /dev/null
+++ b/flang/test/Lower/OpenMP/declare-target-modfile.f90
@@ -0,0 +1,31 @@
+! Cross-TU propagation of `declare target` on a module variable via .mod files.
+
+! RUN: rm -rf %t && split-file %s %t
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -module-dir %t %t/m.f90 -o - > /dev/null
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -J %t %t/use.f90 -o - | FileCheck %s
+
+! First RUN builds dt_mod.mod.
+! Second RUN lowers a consumer that only USE-associates the module. The
+! declaration emitted there must carry omp.declare_target, recovered from the
+! .mod file. Without it the global is treated as an ordinary host global and is
+! given internal linkage for the device, which replaces the definition with an
+! undefined local copy and silently yields wrong results.
+
+!--- m.f90
+module dt_mod
+  implicit none
+  integer :: dt_x
+  !$omp declare target(dt_x)
+end module dt_mod
+
+!--- use.f90
+subroutine use_dt_mod(out)
+  use dt_mod
+  implicit none
+  integer, intent(out) :: out
+  !$omp target map(tofrom: out)
+    out = dt_x
+  !$omp end target
+end subroutine use_dt_mod
+
+! CHECK: fir.global @_QMdt_modEdt_x {omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to), automap = false>} : i32

``````````

</details>


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