[llvm-branch-commits] [mlir] [flang][OpenMP] Translate OpenMP scopes when compiling for target device (PR #130078)

[Sergio Afonso via llvm-branch-commits]

================
@@ -5255,6 +5283,51 @@ convertTargetOpsInNest(Operation *op, llvm::IRBuilderBase &builder,
               return WalkResult::interrupt();
             return WalkResult::skip();
           }
+
+          // Non-target ops might nest target-related ops, therefore, we
+          // translate them as non-OpenMP scopes. Translating them is needed by
+          // nested target-related ops since they might LLVM values defined in
+          // their parent non-target ops.
+          if (isa<omp::OpenMPDialect>(oper->getDialect()) &&
+              oper->getParentOfType<LLVM::LLVMFuncOp>() &&
+              !oper->getRegions().empty()) {
+
+            // TODO Handle other ops with entry block args.
+            llvm::TypeSwitch<Operation &>(*oper)
+                .Case([&](omp::WsloopOp wsloopOp) {
+                  forwardPrivateArgs(wsloopOp, moduleTranslation);
+                  forwardReductionArgs(wsloopOp, moduleTranslation);
+                })
+                .Case([&](omp::ParallelOp parallelOp) {
+                  forwardPrivateArgs(parallelOp, moduleTranslation);
+                  forwardReductionArgs(parallelOp, moduleTranslation);
+                })
+                .Case([&](omp::TaskOp taskOp) {
+                  forwardPrivateArgs(taskOp, moduleTranslation);
+                });
+
+            if (auto loopNest = dyn_cast<omp::LoopNestOp>(oper)) {
+              for (auto iv : loopNest.getIVs()) {
+                // Create fake allocas just to maintain IR validity.
+                moduleTranslation.mapValue(
+                    iv, builder.CreateAlloca(
+                            moduleTranslation.convertType(iv.getType())));
----------------
skatrak wrote:

Yes, I meant that in other places we need to call `findAllocaInsertPoint` before adding new allocas. That will either return the entry block of the function or an alternative spot, if an `OpenMPAllocaStackFrame` was introduced by a parent operation.

So, the current insertion point there will always be valid, as you say, but maybe it wouldn't be the right place for allocas specifically. I'm not that familiar with this, so I may be wrong. At the end of the day operations created here are later removed from the LLVM module, so maybe this doesn't even matter.

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