[flang-commits] [flang] [llvm] [OpenMPIRBuilder] Don't drop debug info for target region. (PR #80692)

[via flang-commits]

https://github.com/abidh created https://github.com/llvm/llvm-project/pull/80692

When an outlined function is generated for omp target region, a corresponding DISubprogram was not being generated. This resulted in all the debug information for the target region being dropped.

This commit adds DISubprogram for the outlined function if there is one available for the parent function. It also updates the current debug location so that the right scope is used for the entries in the outlined function.

With this change in place, I can set source line breakpoint in target region and run to them in debugger.

>From 116741bde587b001e2e2a456cb9e14b08db3a538 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 2 Feb 2024 12:16:52 -0600
Subject: [PATCH] [OpenMPIRBuilder] Don't drop debug info for target region.

When an outlined function is generated for omp target region,
a corresponding DISubprogram was not being generated. This resulted in
all the debug information for the target region being dropped.

This commit adds DISubprogram for the outlined function if there is one
available for the parent function. It also updates the current debug
location so that the right scope is used for the entries in the outlined
function.

With this change in place, I can set source line breakpoint in target
region and run to them in debugger.
---
 flang/test/Lower/OpenMP/target-debug.f90      | 18 ++++++++++
 llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp     | 36 +++++++++++++++++--
 .../Frontend/OpenMPIRBuilderTest.cpp          |  4 +++
 3 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100644 flang/test/Lower/OpenMP/target-debug.f90

diff --git a/flang/test/Lower/OpenMP/target-debug.f90 b/flang/test/Lower/OpenMP/target-debug.f90
new file mode 100644
index 0000000000000..c9843eba391d7
--- /dev/null
+++ b/flang/test/Lower/OpenMP/target-debug.f90
@@ -0,0 +1,18 @@
+!RUN: %flang_fc1 -triple amdgcn-amd-amdhsa %s -debug-info-kind=line-tables-only -fopenmp -fopenmp-is-target-device -emit-llvm -o - | FileCheck %s
+program test
+implicit none
+
+  integer(kind = 4) :: a, b, c, d
+
+  !$omp target map(tofrom: a, b, c, d)
+  a = a + 1
+  ! CHECK: !DILocation(line: [[@LINE-1]]
+  b = a + 2
+  ! CHECK: !DILocation(line: [[@LINE-1]]
+  c = a + 3
+  ! CHECK: !DILocation(line: [[@LINE-1]]
+  d = a + 4
+  ! CHECK: !DILocation(line: [[@LINE-1]]
+  !$omp end target
+
+end program test
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 02b333e9ccd56..f0f26c5b8d8b7 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -31,6 +31,7 @@
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
@@ -5025,10 +5026,41 @@ static Function *createOutlinedFunction(
       ParameterTypes.push_back(Arg->getType());
   }
 
+  auto BB = Builder.GetInsertBlock();
+  auto M = BB->getModule();
   auto FuncType = FunctionType::get(Builder.getVoidTy(), ParameterTypes,
                                     /*isVarArg*/ false);
-  auto Func = Function::Create(FuncType, GlobalValue::InternalLinkage, FuncName,
-                               Builder.GetInsertBlock()->getModule());
+  auto Func =
+      Function::Create(FuncType, GlobalValue::InternalLinkage, FuncName, M);
+
+  // If there's a DISubprogram associated with current function, then
+  // generate one for the outlined function.
+  if (Function *parentFunc = BB->getParent()) {
+    if (DISubprogram *SP = parentFunc->getSubprogram()) {
+      DICompileUnit *CU = SP->getUnit();
+      DIBuilder DB(*M, true, CU);
+      DebugLoc DL = Builder.getCurrentDebugLocation();
+      // TODO: We are using nullopt for arguments at the moment. This will need
+      // to be updated when debug data is being generated for variables.
+      DISubroutineType *Ty =
+          DB.createSubroutineType(DB.getOrCreateTypeArray(std::nullopt));
+      DISubprogram::DISPFlags SPFlags = DISubprogram::SPFlagDefinition |
+                                        DISubprogram::SPFlagOptimized |
+                                        DISubprogram::SPFlagLocalToUnit;
+
+      DISubprogram *OutlinedSP = DB.createFunction(
+          CU, FuncName, FuncName, SP->getFile(), DL.getLine(), Ty, DL.getLine(),
+          DINode::DIFlags::FlagArtificial, SPFlags);
+
+      // Attach subprogram to the function.
+      Func->setSubprogram(OutlinedSP);
+      // Update the CurrentDebugLocation in the builder so that right scope
+      // is used for things inside outlined function.
+      Builder.SetCurrentDebugLocation(
+          DILocation::get(Func->getContext(), DL.getLine(), DL.getCol(),
+                          OutlinedSP, DL.getInlinedAt()));
+    }
+  }
 
   // Save insert point.
   auto OldInsertPoint = Builder.saveIP();
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index e79d0bb2f65ae..c5b5d12840c4d 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -5827,6 +5827,10 @@ TEST_F(OpenMPIRBuilderTest, TargetRegion) {
   BasicBlock *FallbackBlock = Branch->getSuccessor(0);
   Iter = FallbackBlock->rbegin();
   CallInst *FCall = dyn_cast<CallInst>(&*(++Iter));
+  // 'F' has a dummy DISubprogram which causes OutlinedFunc to also
+  // have a DISubprogram. In this case, the call to OutlinedFunc needs
+  // to have a debug loc, otherwise verifier will complain.
+  FCall->setDebugLoc(DL);
   EXPECT_NE(FCall, nullptr);
 
   // Check that the correct aguments are passed in