[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));
+    mlir::LLVM::DISubroutineTypeAttr spTy =
+        mlir::LLVM::DISubroutineTypeAttr::get(context, CC, types);
+    auto spAttr = mlir::LLVM::DISubprogramAttr::get(
+        context, recId, /*isRecSelf=*/true, Id, compilationUnit, Scope, Name,
+        Name, funcFileAttr, line, line, flags, spTy, /*retainedNodes=*/{},
+        /*annotations=*/{});
+
+    // Make sure that information about the imported modules in copied from the
+    // parent function.
+    llvm::SmallVector<mlir::LLVM::DINodeAttr> OpEntities;
+    for (mlir::LLVM::DINodeAttr N : entities) {
+      if (auto entity = mlir::dyn_cast<mlir::LLVM::DIImportedEntityAttr>(N)) {
+        auto importedEntity = mlir::LLVM::DIImportedEntityAttr::get(
+            context, llvm::dwarf::DW_TAG_imported_module, spAttr,
+            entity.getEntity(), fileAttr, /*line=*/1, /*name=*/nullptr,
+            /*elements*/ {});
+        OpEntities.push_back(importedEntity);
+      }
+    }
+
+    Id = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
----------------
skatrak wrote:

I wonder whether it's actually necessary to create new attributes to hold the same constant value. I might be wrong on that, just thinking that MLIR attributes are already uniqued at the context level, so my impression is that this would just end up pointing to the same thing in the end. But I may have misunderstood that, so not a request to change anything.

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