[flang-commits] [flang] [flang][debug] Generate DISubprogramAttr for omp::TargetOp. (PR #138039)

[Sergio Afonso via flang-commits]

================
@@ -510,8 +542,73 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
       subTypeAttr, entities, /*annotations=*/{});
   funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
 
+  /* When we process the DeclareOp inside the OpenMP target region, all the
+     variables get the DISubprogram of the parent function of the target op as
+     the scope. In the codegen (to llvm ir), OpenMP target op results in the
+     creation of a separate function. As the variables in the debug info have
+     the DISubprogram of the parent function as the scope, the variables
+     need to be updated at codegen time to avoid verification failures.
+
+     This updating after the fact becomes more and more difficult when types
+     are dependent on local variables like in the case of variable size arrays
+     or string. We not only have to generate new variables but also new types.
+     We can avoid this problem by generating a DISubprogramAttr here for the
+     target op and make sure that all the variables inside the target region
+     get the correct scope in the first place. */
+  funcOp.walk([&](mlir::omp::TargetOp targetOp) {
+    unsigned line = getLineFromLoc(targetOp.getLoc());
+    mlir::StringAttr Name =
+        getTargetFunctionName(context, targetOp.getLoc(), funcOp.getName());
+    mlir::LLVM::DISubprogramFlags flags =
+        mlir::LLVM::DISubprogramFlags::Definition |
+        mlir::LLVM::DISubprogramFlags::LocalToUnit;
+    if (isOptimized)
+      flags = flags | mlir::LLVM::DISubprogramFlags::Optimized;
+
+    mlir::DistinctAttr recId =
+        mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+    mlir::DistinctAttr Id =
+        mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+    llvm::SmallVector<mlir::LLVM::DITypeAttr> types;
+    types.push_back(mlir::LLVM::DINullTypeAttr::get(context));
----------------
skatrak wrote:

In that case, I'd suggest just passing a single `DINullTypeAttr` (as if it was a parameter-less void function) and then adding a comment explaining what you're saying, basically stating that figuring out the actual argument list of the outlined function at that point is non-trivial and we don't actually need the function type information to be correct to produce the right dwarf. I think this was your original approach, minus the comment, so sorry about the back and forth before agreeing with you! 😄

That way we don't do any work to fill out the argument types incorrectly anyways, and we keep this decision documented in case we later need to actually fill the argument types properly.

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