[flang][debug] Generate DISubprogramAttr for omp::TargetOp.

flang/lib/Optimizer/Transforms/AddDebugInfo.cpp

@@ -34,6 +34,7 @@
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,6 +104,37 @@ bool debugInfoIsAlreadySet(mlir::Location loc) {
   return false;
 }
 
+// Generates the name for the artificial DISubprogram that we are going to
+// generate for omp::TargetOp. Its logic is borrowed from
+// getTargetEntryUniqueInfo and
+// TargetRegionEntryInfo::getTargetRegionEntryFnName to generate the same name.
+// But even if there was a slight mismatch, it is not a problem because this
+// name is artificial and not important to debug experience.
+mlir::StringAttr getTargetFunctionName(mlir::MLIRContext *context,
+                                       mlir::Location Loc,
+                                       llvm::StringRef parentName) {
+  auto fileLoc = Loc->findInstanceOf<mlir::FileLineColLoc>();
+
+  assert(fileLoc && "No file found from location");
+  llvm::StringRef fileName = fileLoc.getFilename().getValue();
+
+  llvm::sys::fs::UniqueID id;
+  uint64_t line = fileLoc.getLine();
+  size_t fileId;
+  size_t deviceId;
+  if (auto ec = llvm::sys::fs::getUniqueID(fileName, id)) {
+    fileId = llvm::hash_value(fileName.str());
+    deviceId = 0xdeadf17e;
+  } else {
+    fileId = id.getFile();
+    deviceId = id.getDevice();
+  }
+  return mlir::StringAttr::get(
+      context,
+      std::string(llvm::formatv("__omp_offloading_{0:x-}_{1:x-}_{2}_l{3}",
+                                deviceId, fileId, parentName, line)));
+}
+
 } // namespace
 
 bool AddDebugInfoPass::createCommonBlockGlobal(
@@ -445,13 +477,88 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
                                   line - 1, false);
   }
 
+  auto addTargetOpDISP = [&](bool lineTableOnly,
+                             const llvm::SmallVector<mlir::LLVM::DINodeAttr>
+                                 &entities) {
+    // 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 id =
+          mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+      llvm::SmallVector<mlir::LLVM::DITypeAttr> types;
+      types.push_back(mlir::LLVM::DINullTypeAttr::get(context));
+      for (auto arg : targetOp.getRegion().getArguments()) {
+        auto tyAttr = typeGen.convertType(fir::unwrapRefType(arg.getType()),
+                                          fileAttr, cuAttr, /*declOp=*/nullptr);
+        types.push_back(tyAttr);
+      }
+      CC = llvm::dwarf::getCallingConvention("DW_CC_normal");
+      mlir::LLVM::DISubroutineTypeAttr spTy =
+          mlir::LLVM::DISubroutineTypeAttr::get(context, CC, types);
+      if (lineTableOnly) {
+        auto spAttr = mlir::LLVM::DISubprogramAttr::get(
+            context, id, compilationUnit, Scope, name, name, funcFileAttr, line,
+            line, flags, spTy, /*retainedNodes=*/{}, /*annotations=*/{});
+        targetOp->setLoc(builder.getFusedLoc({targetOp.getLoc()}, spAttr));
+        return;
+      }
+      mlir::DistinctAttr recId =
+          mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+      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 is copied in the
+      // new 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));
+      spAttr = mlir::LLVM::DISubprogramAttr::get(
+          context, recId, /*isRecSelf=*/false, id, compilationUnit, Scope, name,
+          name, funcFileAttr, line, line, flags, spTy, opEntities,
+          /*annotations=*/{});
+      targetOp->setLoc(builder.getFusedLoc({targetOp.getLoc()}, spAttr));
+    });
+  };
+
   // Don't process variables if user asked for line tables only.
   if (debugLevel == mlir::LLVM::DIEmissionKind::LineTablesOnly) {
     auto spAttr = mlir::LLVM::DISubprogramAttr::get(
         context, id, compilationUnit, Scope, funcName, fullName, funcFileAttr,
         line, line, subprogramFlags, subTypeAttr, /*retainedNodes=*/{},
         /*annotations=*/{});
     funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
+    addTargetOpDISP(true, {});
     return;
   }
 
@@ -509,9 +616,18 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
       funcName, fullName, funcFileAttr, line, line, subprogramFlags,
       subTypeAttr, entities, /*annotations=*/{});
   funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
+  addTargetOpDISP(false, entities);
 
   funcOp.walk([&](fir::cg::XDeclareOp declOp) {
-    handleDeclareOp(declOp, fileAttr, spAttr, typeGen, symbolTable);
+    mlir::LLVM::DISubprogramAttr spTy = spAttr;
+    if (auto tOp = declOp->getParentOfType<mlir::omp::TargetOp>()) {
+      if (auto fusedLoc = llvm::dyn_cast<mlir::FusedLoc>(tOp.getLoc())) {
+        if (auto sp = llvm::dyn_cast<mlir::LLVM::DISubprogramAttr>(
+                fusedLoc.getMetadata()))
+          spTy = sp;
+      }
+    }
+    handleDeclareOp(declOp, fileAttr, spTy, typeGen, symbolTable);
   });
   // commonBlockMap ensures that we don't create multiple DICommonBlockAttr of
   // the same name in one function. But it is ok (rather required) to create

flang/test/Transforms/debug-omp-target-op-1.fir

@@ -0,0 +1,40 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
+// RUN: fir-opt --add-debug-info="debug-level=LineTablesOnly" --mlir-print-debuginfo %s | FileCheck %s --check-prefix=LINETABLE
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
+  func.func @_QQmain() attributes {fir.bindc_name = "test"} {
+    %c13_i32 = arith.constant 13 : i32
+    %c12_i32 = arith.constant 12 : i32
+    %c6_i32 = arith.constant 6 : i32
+    %c1_i32 = arith.constant 1 : i32
+    %c5_i32 = arith.constant 5 : i32
+    %0 = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFEx"} loc(#loc1)
+    %1 = fircg.ext_declare %0 {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc1)
+    %2 = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFEy"} loc(#loc2)
+    %3 = fircg.ext_declare %2 {uniq_name = "_QFEy"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc2)
+    %4 = omp.map.info var_ptr(%1 : !fir.ref<i32>, i32) map_clauses(tofrom) capture(ByRef) -> !fir.ref<i32> {name = "x"}
+    %5 = omp.map.info var_ptr(%3 : !fir.ref<i32>, i32) map_clauses(tofrom) capture(ByRef) -> !fir.ref<i32> {name = "y"}
+    omp.target map_entries(%4 -> %arg0, %5 -> %arg1 : !fir.ref<i32>, !fir.ref<i32>) {
+      %16 = fircg.ext_declare %arg0 {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc3)
+      %17 = fircg.ext_declare %arg1 {uniq_name = "_QFEy"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc4)
+      omp.terminator
+    } loc(#loc5)
+    return
+  }
+}
+#loc1 = loc("test.f90":1:1)
+#loc2 = loc("test.f90":3:1)
+#loc3 = loc("test.f90":7:1)
+#loc4 = loc("test.f90":8:1)
+#loc5 = loc("test.f90":6:1)
+
+// CHECK: #[[SP:.*]] = #llvm.di_subprogram<{{.*}}name = "test"{{.*}}>
+// CHECK: #[[SP1:.*]] = #llvm.di_subprogram<{{.*}}name = "__omp_offloading_{{.*}}_QQmain_l6"{{.*}}line = 6{{.*}}subprogramFlags = "LocalToUnit|Definition"{{.*}}>
+// CHECK: #llvm.di_local_variable<scope = #[[SP]], name = "x"{{.*}}line = 1, type = #[[TY:.*]]>
+// CHECK: #llvm.di_local_variable<scope = #[[SP]], name = "y"{{.*}}line = 3, type = #[[TY]]>
+// CHECK: #llvm.di_local_variable<scope = #[[SP1]], name = "x"{{.*}}line = 7, type = #[[TY]]>
+// CHECK: #llvm.di_local_variable<scope = #[[SP1]], name = "y"{{.*}}line = 8, type = #[[TY]]>
+
+// LINETABLE: #[[SP:.*]] = #llvm.di_subprogram<{{.*}}name = "test"{{.*}}>
+// LINETABLE: #[[SP1:.*]] = #llvm.di_subprogram<{{.*}}name = "__omp_offloading_{{.*}}_QQmain_l6"{{.*}}line = 6{{.*}}subprogramFlags = "LocalToUnit|Definition"{{.*}}>
+// LINETABLE-NOT: #llvm.di_local_variable

flang/test/Transforms/debug-omp-target-op-2.fir

@@ -0,0 +1,53 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
+  func.func @fn_(%arg0: !fir.ref<!fir.array<?x?xi32>> {fir.bindc_name = "b"}, %arg1: !fir.ref<i32> {fir.bindc_name = "c"}, %arg2: !fir.ref<i32> {fir.bindc_name = "d"}) {
+    %c1 = arith.constant 1 : index
+    %c0 = arith.constant 0 : index
+    %0 = fir.alloca i32
+    %1 = fir.alloca i32
+    %2 = fir.undefined !fir.dscope
+    %3 = fircg.ext_declare %arg1 dummy_scope %2 {uniq_name = "_QFfnEc"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32> loc(#loc2)
+    %4 = fircg.ext_declare %arg2 dummy_scope %2 {uniq_name = "_QFfnEd"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32> loc(#loc3)
+    %5 = fir.load %3 : !fir.ref<i32>
+    %6 = fir.convert %5 : (i32) -> index
+    %9 = fir.load %4 : !fir.ref<i32>
+    %10 = fir.convert %9 : (i32) -> index
+    %15 = fircg.ext_declare %arg0(%6, %10) dummy_scope %2 {uniq_name = "_QFfnEb"} : (!fir.ref<!fir.array<?x?xi32>>, index, index, !fir.dscope) -> !fir.ref<!fir.array<?x?xi32>> loc(#loc4)
+    %16 = fircg.ext_embox %15(%6, %10) : (!fir.ref<!fir.array<?x?xi32>>, index, index) -> !fir.box<!fir.array<?x?xi32>>
+    %17:3 = fir.box_dims %16, %c0 : (!fir.box<!fir.array<?x?xi32>>, index) -> (index, index, index)
+    %18 = arith.subi %17#1, %c1 : index
+    %19 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%18 : index) extent(%17#1 : index) stride(%17#2 : index) start_idx(%c1 : index) {stride_in_bytes = true}
+    %20 = arith.muli %17#2, %17#1 : index
+    %21:3 = fir.box_dims %16, %c1 : (!fir.box<!fir.array<?x?xi32>>, index) -> (index, index, index)
+    %22 = arith.subi %21#1, %c1 : index
+    %23 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%22 : index) extent(%21#1 : index) stride(%20 : index) start_idx(%c1 : index) {stride_in_bytes = true}
+    %24 = omp.map.info var_ptr(%15 : !fir.ref<!fir.array<?x?xi32>>, i32) map_clauses(tofrom) capture(ByRef) bounds(%19, %23) -> !fir.ref<!fir.array<?x?xi32>> {name = "b"}
+    %25 = omp.map.info var_ptr(%1 : !fir.ref<i32>, i32) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByCopy) -> !fir.ref<i32> {name = ""}
+    %26 = omp.map.info var_ptr(%0 : !fir.ref<i32>, i32) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByCopy) -> !fir.ref<i32> {name = ""}
+    omp.target map_entries(%24 -> %arg3, %25 -> %arg4, %26 -> %arg5 : !fir.ref<!fir.array<?x?xi32>>, !fir.ref<i32>, !fir.ref<i32>) {
+      %27 = fir.load %arg5 : !fir.ref<i32>
+      %28 = fir.load %arg4 : !fir.ref<i32>
+      %29 = fir.convert %27 : (i32) -> index
+      %31 = fir.convert %28 : (i32) -> index
+      %37 = fircg.ext_declare %arg3(%29, %31) {uniq_name = "_QFfnEb"} : (!fir.ref<!fir.array<?x?xi32>>, index, index) -> !fir.ref<!fir.array<?x?xi32>> loc(#loc5)
+      omp.terminator
+    } loc(#loc6)
+    return
+  } loc(#loc7)
+}
+#loc1 = loc("test.f90":1:1)
+#loc2 = loc("test.f90":3:1)
+#loc3 = loc("test.f90":7:1)
+#loc4 = loc("test.f90":8:1)
+#loc5 = loc("test.f90":6:1)
+#loc6 = loc("test.f90":16:1)
+#loc7 = loc("test.f90":26:1)
+
+
+// Test that variable size arrays inside target regions get their own
+// compiler generated variables for size.
+
+// CHECK: #[[SP:.*]] = #llvm.di_subprogram<{{.*}}name = "__omp_offloading_{{.*}}_fn__l16"{{.*}}>
+// CHECK: #llvm.di_local_variable<scope = #[[SP]], name = "._QFfnEb1"{{.*}}>
+// CHECK: #llvm.di_local_variable<scope = #[[SP]], name = "._QFfnEb2"{{.*}}>