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

[Kareem Ergawy]

================
@@ -5315,6 +5320,46 @@ 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 need LLVM values defined
+          // in their parent non-target ops.
+          if (isa<omp::OpenMPDialect>(oper->getDialect()) &&
+              oper->getParentOfType<LLVM::LLVMFuncOp>() &&
+              !oper->getRegions().empty()) {
----------------
ergawy wrote:

Thanks for the example. However, this is not an issue since when translating the surrounding region, the LLVM value corresponding to the original `n` value will be resolved:
```llvm
omp.parallel.fake.region:
  %2 = load i32, ptr @_QFtestEn, align 4
  %3 = sext i32 %2 to i64
  %4 = sub i64 %3, 1
  %5 = sub i64 %4, 0
  %6 = add i64 %5, 1
  %7 = mul i64 1, %6
  %8 = mul i64 %7, 4
  br label %omp.region.cont
```
So the translation happens without issues.

I think we should keep things simple and not add the extra translation for non-region ops since there does not seem to be need for that, at least at the moment. I think it is not much more effort, the main reason is to simplify things even by a little bit. Let me know if you disagree.

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