Handle review comments.

The new way to set debug location can better handle empty basic block or function. A testcase is added as well.

Author: abidh

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

@@ -4985,19 +4985,23 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
   // The current debug location already has the DISubprogram for the outlined
   // function that will be created for the target op. We save it here so that
   // we can set it on the outlined function.
-  llvm::DebugLoc OutlinedFnLoc = builder.getCurrentDebugLocation();
+  llvm::DebugLoc outlinedFnLoc = builder.getCurrentDebugLocation();
+  if (failed(checkImplementationStatus(opInst)))
+    return failure();
+
   // During the handling of target op, we will generate instructions in the
-  // parent function like call to oulined function or branch to new BasicBlock.
-  // We set the debug location here to parent function so that those get the
-  // correct debug locations. For outlined functions, the normal MLIR op
+  // parent function like call to the oulined function or branch to a new
+  // BasicBlock. We set the debug location here to parent function so that those
+  // get the correct debug locations. For outlined functions, the normal MLIR op
   // conversion will automatically pick the correct location.
   llvm::BasicBlock *parentBB = builder.GetInsertBlock();
-  if (parentBB && !parentBB->empty())
-    builder.SetCurrentDebugLocation(parentBB->back().getDebugLoc());
-  else
-    builder.SetCurrentDebugLocation(llvm::DebugLoc());
-  if (failed(checkImplementationStatus(opInst)))
-    return failure();
+  assert(parentBB);
+  llvm::Function *parentLLVMFn = parentBB->getParent();
+  assert(parentLLVMFn);
+  if (llvm::DISubprogram *SP = parentLLVMFn->getSubprogram())
+    builder.SetCurrentDebugLocation(llvm::DILocation::get(
+        parentLLVMFn->getContext(), outlinedFnLoc.getLine(),
+        outlinedFnLoc.getCol(), SP, outlinedFnLoc.getInlinedAt()));
 
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
   bool isTargetDevice = ompBuilder->Config.isTargetDevice();
@@ -5092,8 +5096,8 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
     assert(llvmParentFn && llvmOutlinedFn &&
            "Both parent and outlined functions must exist at this point");
 
-    if (OutlinedFnLoc && llvmParentFn->getSubprogram())
-      llvmOutlinedFn->setSubprogram(OutlinedFnLoc->getScope()->getSubprogram());
+    if (outlinedFnLoc && llvmParentFn->getSubprogram())
+      llvmOutlinedFn->setSubprogram(outlinedFnLoc->getScope()->getSubprogram());
 
     if (auto attr = llvmParentFn->getFnAttribute("target-cpu");
         attr.isStringAttribute())

mlir/test/Target/LLVMIR/omptarget-debug-empty.mlir

@@ -0,0 +1,27 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+module attributes {omp.is_target_device = false} {
+  llvm.func @test() {
+    omp.target {
+      omp.terminator
+    }  loc(#loc4)
+    llvm.return
+  }  loc(#loc3)
+}
+#file = #llvm.di_file<"target.f90" in "">
+#cu = #llvm.di_compile_unit<id = distinct[0]<>,
+ sourceLanguage = DW_LANG_Fortran95, file = #file, isOptimized = false,
+ emissionKind = Full>
+#sp_ty = #llvm.di_subroutine_type<callingConvention = DW_CC_normal>
+#sp = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #cu, scope = #file,
+ name = "_QQmain", file = #file, subprogramFlags = "Definition", type = #sp_ty>
+#sp1 = #llvm.di_subprogram<id = distinct[2]<>, compileUnit = #cu, scope = #file,
+ name = "__omp_offloading_target", file = #file, subprogramFlags = "Definition",
+ type = #sp_ty>
+#loc1 = loc("target.f90":1:1)
+#loc2 = loc("target.f90":46:3)
+#loc3 = loc(fused<#sp>[#loc1])
+#loc4 = loc(fused<#sp1>[#loc2])
+
+// CHECK: ![[SP:.*]] = {{.*}}!DISubprogram(name: "__omp_offloading_target"{{.*}})
+