[Mlir-commits] [mlir] [OpenMP][Flang] Add "IsolatedFromAbove" trait to omp.target (PR #67164)

[llvmlistbot at llvm.org]

================
@@ -2400,6 +2420,52 @@ genEnterExitDataOp(Fortran::lower::AbstractConverter &converter,
                                    deviceOperand, nowaitAttr, mapOperands);
 }
 
+static void genBodyOfTargetOp(
+    Fortran::lower::AbstractConverter &converter, mlir::omp::TargetOp &targetOp,
+    const llvm::SmallVector<mlir::Type> &mapSymTypes,
+    const llvm::SmallVector<mlir::Location> &mapSymLocs,
+    const llvm::SmallVector<const Fortran::semantics::Symbol *> &mapSymbols,
+    const mlir::Location &currentLocation) {
+  assert(mapSymTypes.size() == mapSymLocs.size() &&
+         mapSymTypes.size() == mapSymbols.size());
+
+  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+  mlir::Region &region = targetOp.getRegion();
+
+  firOpBuilder.createBlock(&region, {}, mapSymTypes, mapSymLocs);
+  firOpBuilder.create<mlir::omp::TerminatorOp>(currentLocation);
+  firOpBuilder.setInsertionPointToStart(&region.front());
+
+  unsigned argIndex = 0;
+  for (const Fortran::semantics::Symbol *sym : mapSymbols) {
+    const mlir::BlockArgument &arg = region.front().getArgument(argIndex);
+    fir::ExtendedValue extVal = converter.getSymbolExtendedValue(*sym);
+    mlir::Value val = fir::getBase(arg);
+    extVal.match(
+        [&](const fir::BoxValue &v) {
+          converter.bindSymbol(*sym, fir::BoxValue(val, v.getLBounds(),
+                                                   v.getExplicitParameters(),
+                                                   v.getExplicitExtents()));
+        },
+        [&](const fir::MutableBoxValue &v) {
+          converter.bindSymbol(*sym,
+                               fir::MutableBoxValue(val, v.getLBounds(),
+                                                    v.getMutableProperties()));
+        },
+        [&](const fir::ArrayBoxValue &v) {
+          converter.bindSymbol(*sym, fir::ArrayBoxValue(val, v.getExtents(),
+                                                        v.getLBounds(),
+                                                        v.getSourceBox()));
+        },
+        [&](const fir::UnboxedValue &v) { converter.bindSymbol(*sym, val); },
+        [&](const auto &) {
+          TODO(converter.getCurrentLocation(),
+               "target map clause operand unsupported type");
----------------
jeanPerier wrote:

@TIFitis, the source of the `op using value defined outside the region` issue is here, and is not HLFIR specific.

You are mapping the bounds and lengths SSA value created outside of the target op to the symbols, so any lowering code that need to use the bounds/lengths will use them and create invalid IR.

You need to pass all those bounds/length values via block argument somehow (for the ones that are constant, you can re-create them inside the target region).

They are several ways to pass these values, you can pass the entity as a "whole" fir.box/fir.boxchar and extract back all the info inside the region (a bit like it is done for internal procedures), or pass the non constant extents/lengths one by one.

Another solution would be to first lower all your code in some non isolated from above region, and to look for all access to region defined outside of this region and to make them block arguments of a new region where you would clone your lowered code.

Here are a few example that will fail without HLFIR (HLFIR just happens to use all the extents/lengths right away in the hlfir.declare for the captured variable):

```
subroutine omp_target_implicit
   integer :: a(1024)
   !$omp target
      a = 10
   !$omp end target
end subroutine omp_target_implicit
```
or

```
subroutine omp_target_implicit2(n)
   integer :: n
   integer :: a(n, 1024)
   !$omp target
      a(2,3) = 10
   !$omp end target
end subroutine omp_target_implicit2
```


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