[flang-commits] [flang] [flang][OpenMP] Fix declare target attribute for USE-associated module globals (PR #218339)

[via flang-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-openmp

Author: Urvi Rav (ravurvi20)

<details>
<summary>Changes</summary>


Fixes #<!-- -->218327 
#### Summary

When a module variable is marked `!$omp declare target` and used in a separately compiled file, Flang was dropping the `declare target` attribute.

Because of this, the device global was treated as a normal global and got internalized. This broke the device-to-host data copy, so `target update from(...)` returned the stale host value instead of the value computed on the device.

#### Fix

When lowering a USE-associated module variable, preserve the `omp.declare_target` attribute from the original symbol.

The `declare target` information is preserved in the `.mod` file, so it can be restored when the variable is lowered in the USE-ing file.

Before -
```
fir.global @<!-- -->_QMdt_modEarr {alignment = 64 : i64} : !fir.array<100xi32>
```
After -
```
fir.global @<!-- -->_QMdt_modEarr {alignment = 64 : i64, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to), automap = false>} : !fir.array<100xi32>
```

#### Testing

* Added `declare-target-use-associated-global.f90` to verify that the external `fir.global` keeps the `omp.declare_target` attribute.


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


2 Files Affected:

- (modified) flang/lib/Lower/ConvertVariable.cpp (+45) 
- (added) flang/test/Lower/OpenMP/declare-target-use-associated-global.f90 (+28) 


``````````diff
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index a808905850922..f48968083dc5a 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -46,6 +46,7 @@
 #include "flang/Semantics/tools.h"
 #include "flang/Semantics/type.h"
 #include "mlir/Dialect/OpenACC/OpenACC.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
@@ -194,6 +195,49 @@ static void attachAccDeclareAttribute(fir::FirOpBuilder &builder,
                                                 builder.getContext(), clause)));
 }
 
+/// Copy the `omp.declare_target` attribute onto an external global declaration
+/// created for a USE-associated module variable. The directive lives in the
+/// defining module, so the USE-ing unit must re-attach it here; otherwise the
+/// global is internalized on the device and loses its device-resident copy.
+static void
+attachDeclareTargetAttribute(fir::FirOpBuilder &builder, fir::GlobalOp global,
+                             const Fortran::semantics::Symbol &sym) {
+  using Flag = Fortran::semantics::Symbol::Flag;
+  const Fortran::semantics::Symbol &ultimate = sym.GetUltimate();
+  if (!ultimate.test(Flag::OmpDeclareTarget))
+    return;
+  auto declareTargetOp =
+      llvm::dyn_cast<mlir::omp::DeclareTargetInterface>(global.getOperation());
+  if (!declareTargetOp || declareTargetOp.isDeclareTarget())
+    return;
+
+  mlir::omp::DeclareTargetDeviceType deviceType =
+      mlir::omp::DeclareTargetDeviceType::any;
+  mlir::omp::DeclareTargetCaptureClause captureClause =
+      mlir::omp::DeclareTargetCaptureClause::to;
+  if (const auto *details =
+          ultimate.detailsIf<Fortran::semantics::ObjectEntityDetails>()) {
+    if (const std::optional<Fortran::common::OmpDeviceType> &dt =
+            details->ompDeclTargetDeviceType()) {
+      switch (*dt) {
+      case Fortran::common::OmpDeviceType::Host:
+        deviceType = mlir::omp::DeclareTargetDeviceType::host;
+        break;
+      case Fortran::common::OmpDeviceType::Nohost:
+        deviceType = mlir::omp::DeclareTargetDeviceType::nohost;
+        break;
+      case Fortran::common::OmpDeviceType::Any:
+        deviceType = mlir::omp::DeclareTargetDeviceType::any;
+        break;
+      }
+    }
+    if (details->ompDeclTarget().test(llvm::omp::Clause::OMPC_link))
+      captureClause = mlir::omp::DeclareTargetCaptureClause::link;
+  }
+  declareTargetOp.setDeclareTarget(deviceType, captureClause,
+                                   /*automap=*/false);
+}
+
 /// Create the global op declaration without any initializer
 static fir::GlobalOp declareGlobal(Fortran::lower::AbstractConverter &converter,
                                    const Fortran::lower::pft::Variable &var,
@@ -225,6 +269,7 @@ static fir::GlobalOp declareGlobal(Fortran::lower::AbstractConverter &converter,
       isConstant(ultimate), var.isTarget(), dataAttr,
       /*setDefaultAlignment=*/!isBindC);
   attachAccDeclareAttribute(builder, global, sym);
+  attachDeclareTargetAttribute(builder, global, sym);
   Fortran::lower::declareExternalAccModuleDeclareActionRecipes(converter,
                                                                builder, sym);
   return global;
diff --git a/flang/test/Lower/OpenMP/declare-target-use-associated-global.f90 b/flang/test/Lower/OpenMP/declare-target-use-associated-global.f90
new file mode 100644
index 0000000000000..4da592366aab4
--- /dev/null
+++ b/flang/test/Lower/OpenMP/declare-target-use-associated-global.f90
@@ -0,0 +1,28 @@
+! A module variable marked `declare target` must keep that attribute when it is
+! USE-associated in a separately compiled file. The directive lives in the
+! module, so the USE-ing file must copy the attribute onto the external global
+! it declares. Without it the device pass internalizes the global and
+! `target update` reads stale data.
+
+! RUN: split-file %s %t
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -module-dir %t %t/mod.f90 -o /dev/null
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -fopenmp-is-target-device -module-dir %t %t/use.f90 -o - | FileCheck %s
+
+!--- mod.f90
+module dt_mod
+  implicit none
+  integer :: arr(100)
+  !$omp declare target enter(arr)
+end module
+
+!--- use.f90
+! Here the global is only an external declaration, but it still must carry the
+! declare target attribute.
+! CHECK: fir.global @_QMdt_modEarr {{{.*}}omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to), automap = false>{{.*}}} : !fir.array<100xi32>
+subroutine sub(i)
+  use dt_mod
+  implicit none
+  !$omp declare target
+  integer :: i
+  arr(i) = i
+end subroutine

``````````

</details>


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