[flang-commits] [flang] [mlir] [mlir][debug] Handle DIImportedEntity. (PR #103055)

Abid Qadeer via flang-commits flang-commits at lists.llvm.org
Fri Aug 23 02:02:26 PDT 2024


https://github.com/abidh updated https://github.com/llvm/llvm-project/pull/103055

>From 9db38e2c57010c45a25553139f8a52c57c18039a Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 19 Jul 2024 18:31:48 +0100
Subject: [PATCH 1/5] [mlir][debug] Handle DIImportedEntity.

The `DIImporedEntity` can be used to represent imported entities like
C++'s namespace with using directive or fortran's moudule with use
statement.

This PR adds `DIImportedEntityAttr` and 2-way translation from
`DIImportedEntity` to `DIImportedEntityAttr` and vice versa.

When an entity is imported in a function, the `retainedNodes` field of
the `DISubprogram` contains all the imported nodes. See the C++ code and the
LLVM IR below.

void test() {
    using namespace n1;
 ...
}

!2 = !DINamespace(name: "n1", scope: null)
!16 = distinct !DISubprogram(name: "test", ..., retainedNodes: !19)
!19 = !{!20}
!20 = !DIImportedEntity(tag: DW_TAG_imported_module, scope: !16, entity: !2 ...)

This PR makes sure that the translation from mlir to `retainedNodes`
field happens correctly both ways.

There was no obvious place where I could place the `DIImportedEntityAttr`
in the mlir. I have decided to piggy back on the `FusedLoc` of `FuncOp`
which is already used to carry `DISubprogramAttr`. I am open to other
ideas here.

This PR currently does not handle entities imported in a global scope
but that should be easy to handle in a subsequent PR.
---
 .../mlir/Dialect/LLVMIR/LLVMAttrDefs.td       | 19 +++++++++
 mlir/lib/Target/LLVMIR/DebugImporter.cpp      | 27 ++++++++++++-
 mlir/lib/Target/LLVMIR/DebugImporter.h        |  1 +
 mlir/lib/Target/LLVMIR/DebugTranslation.cpp   | 40 ++++++++++++++++---
 mlir/lib/Target/LLVMIR/DebugTranslation.h     |  1 +
 mlir/test/Target/LLVMIR/Import/debug-info.ll  | 26 ++++++++++++
 mlir/test/Target/LLVMIR/llvmir-debug.mlir     | 33 +++++++++++++++
 7 files changed, 141 insertions(+), 6 deletions(-)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 5d96f506342588..b9f99ef70cc884 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -619,6 +619,25 @@ def LLVM_DINamespaceAttr : LLVM_Attr<"DINamespace", "di_namespace",
   let assemblyFormat = "`<` struct(params) `>`";
 }
 
+//===----------------------------------------------------------------------===//
+// DIImportedEntityAttr
+//===----------------------------------------------------------------------===//
+
+def LLVM_DIImportedEntityAttr : LLVM_Attr<"DIImportedEntity", "di_imported_entity",
+                                           /*traits=*/[], "DINodeAttr"> {
+  let parameters = (ins
+    LLVM_DITagParameter:$tag,
+    OptionalParameter<"DIScopeAttr">:$scope,
+    OptionalParameter<"DINodeAttr">:$entity,
+    OptionalParameter<"DIFileAttr">:$file,
+    OptionalParameter<"unsigned">:$line,
+    OptionalParameter<"StringAttr">:$name,
+    OptionalArrayRefParameter<"DINodeAttr">:$elements
+  );
+
+  let assemblyFormat = "`<` struct(params) `>`";
+}
+
 //===----------------------------------------------------------------------===//
 // DISubrangeAttr
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
index 1817c1271b43ee..77fd292a5cca12 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -40,10 +40,18 @@ Location DebugImporter::translateFuncLocation(llvm::Function *func) {
   // Add a fused location to link the subprogram information.
   StringAttr funcName = StringAttr::get(context, subprogram->getName());
   StringAttr fileName = StringAttr::get(context, subprogram->getFilename());
-  return FusedLocWith<DISubprogramAttr>::get(
+  auto loc = FusedLocWith<DISubprogramAttr>::get(
       {NameLoc::get(funcName),
        FileLineColLoc::get(fileName, subprogram->getLine(), /*column=*/0)},
       translate(subprogram), context);
+  if (subprogram->getRetainedNodes().empty())
+    return loc;
+  llvm::SmallVector<mlir::Attribute> entities;
+  for (auto node : subprogram->getRetainedNodes())
+    entities.push_back(translate(node));
+
+  auto entitiesAttr = mlir::ArrayAttr::get(context, entities);
+  return FusedLocWith<mlir::ArrayAttr>::get(loc, entitiesAttr, context);
 }
 
 //===----------------------------------------------------------------------===//
@@ -208,6 +216,21 @@ DINamespaceAttr DebugImporter::translateImpl(llvm::DINamespace *node) {
                               node->getExportSymbols());
 }
 
+DIImportedEntityAttr
+DebugImporter::translateImpl(llvm::DIImportedEntity *node) {
+  SmallVector<DINodeAttr> elements;
+
+  for (llvm::DINode *element : node->getElements()) {
+    assert(element && "expected a non-null element type");
+    elements.push_back(translate(element));
+  }
+
+  return DIImportedEntityAttr::get(
+      context, node->getTag(), translate(node->getScope()),
+      translate(node->getEntity()), translate(node->getFile()), node->getLine(),
+      getStringAttrOrNull(node->getRawName()), elements);
+}
+
 DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
   // Only definitions require a distinct identifier.
   mlir::DistinctAttr id;
@@ -308,6 +331,8 @@ DINodeAttr DebugImporter::translate(llvm::DINode *node) {
       return translateImpl(casted);
     if (auto *casted = dyn_cast<llvm::DIGlobalVariable>(node))
       return translateImpl(casted);
+    if (auto *casted = dyn_cast<llvm::DIImportedEntity>(node))
+      return translateImpl(casted);
     if (auto *casted = dyn_cast<llvm::DILabel>(node))
       return translateImpl(casted);
     if (auto *casted = dyn_cast<llvm::DILexicalBlock>(node))
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.h b/mlir/lib/Target/LLVMIR/DebugImporter.h
index 0e040891ba6c02..cb796676759c39 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.h
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.h
@@ -75,6 +75,7 @@ class DebugImporter {
   DIVariableAttr translateImpl(llvm::DIVariable *node);
   DIModuleAttr translateImpl(llvm::DIModule *node);
   DINamespaceAttr translateImpl(llvm::DINamespace *node);
+  DIImportedEntityAttr translateImpl(llvm::DIImportedEntity *node);
   DIScopeAttr translateImpl(llvm::DIScope *node);
   DISubprogramAttr translateImpl(llvm::DISubprogram *node);
   DISubrangeAttr translateImpl(llvm::DISubrange *node);
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index 95b37e47d04612..93ca3cfc5eba72 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -67,7 +67,25 @@ void DebugTranslation::translate(LLVMFuncOp func, llvm::Function &llvmFunc) {
       func.getLoc()->findInstanceOf<FusedLocWith<LLVM::DISubprogramAttr>>();
   if (!spLoc)
     return;
-  llvmFunc.setSubprogram(translate(spLoc.getMetadata()));
+  llvm::DISubprogram *sp = translate(spLoc.getMetadata());
+  llvmFunc.setSubprogram(sp);
+
+  // Look for any entities that are imported in this function.
+  auto entityLoc =
+      func.getLoc()->findInstanceOf<FusedLocWith<mlir::ArrayAttr>>();
+  if (!entityLoc)
+    return;
+
+  SmallVector<llvm::Metadata *> imported;
+  if (mlir::ArrayAttr arrayAttr = entityLoc.getMetadata()) {
+    for (mlir::Attribute attr : arrayAttr.getValue()) {
+      if (auto ent = dyn_cast_if_present<LLVM::DIImportedEntityAttr>(attr)) {
+        llvm::DINode *node = translate(ent);
+        imported.push_back(node);
+      }
+    }
+  }
+  sp->replaceRetainedNodes(llvm::MDTuple::get(llvmFunc.getContext(), imported));
 }
 
 //===----------------------------------------------------------------------===//
@@ -326,6 +344,18 @@ llvm::DINamespace *DebugTranslation::translateImpl(DINamespaceAttr attr) {
                                 attr.getExportSymbols());
 }
 
+llvm::DIImportedEntity *
+DebugTranslation::translateImpl(DIImportedEntityAttr attr) {
+  SmallVector<llvm::Metadata *> elements;
+  for (DINodeAttr member : attr.getElements())
+    elements.push_back(translate(member));
+
+  return llvm::DIImportedEntity::get(
+      llvmCtx, attr.getTag(), translate(attr.getScope()),
+      translate(attr.getEntity()), translate(attr.getFile()), attr.getLine(),
+      getMDStringOrNull(attr.getName()), llvm::MDNode::get(llvmCtx, elements));
+}
+
 llvm::DISubrange *DebugTranslation::translateImpl(DISubrangeAttr attr) {
   auto getMetadataOrNull = [&](Attribute attr) -> llvm::Metadata * {
     if (!attr)
@@ -388,10 +418,10 @@ llvm::DINode *DebugTranslation::translate(DINodeAttr attr) {
     node = TypeSwitch<DINodeAttr, llvm::DINode *>(attr)
                .Case<DIBasicTypeAttr, DICompileUnitAttr, DICompositeTypeAttr,
                      DIDerivedTypeAttr, DIFileAttr, DIGlobalVariableAttr,
-                     DILabelAttr, DILexicalBlockAttr, DILexicalBlockFileAttr,
-                     DILocalVariableAttr, DIModuleAttr, DINamespaceAttr,
-                     DINullTypeAttr, DIStringTypeAttr, DISubprogramAttr,
-                     DISubrangeAttr, DISubroutineTypeAttr>(
+                     DIImportedEntityAttr, DILabelAttr, DILexicalBlockAttr,
+                     DILexicalBlockFileAttr, DILocalVariableAttr, DIModuleAttr,
+                     DINamespaceAttr, DINullTypeAttr, DIStringTypeAttr,
+                     DISubprogramAttr, DISubrangeAttr, DISubroutineTypeAttr>(
                    [&](auto attr) { return translateImpl(attr); });
 
   if (node && !node->isTemporary())
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.h b/mlir/lib/Target/LLVMIR/DebugTranslation.h
index 16a853736226da..3f28297fc7f594 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.h
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.h
@@ -84,6 +84,7 @@ class DebugTranslation {
   llvm::DIVariable *translateImpl(DIVariableAttr attr);
   llvm::DIModule *translateImpl(DIModuleAttr attr);
   llvm::DINamespace *translateImpl(DINamespaceAttr attr);
+  llvm::DIImportedEntity *translateImpl(DIImportedEntityAttr attr);
   llvm::DIScope *translateImpl(DIScopeAttr attr);
   llvm::DISubprogram *translateImpl(DISubprogramAttr attr);
   llvm::DISubrange *translateImpl(DISubrangeAttr attr);
diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll
index 03c3855a9a324c..77ed6183cebae4 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -792,3 +792,29 @@ define void @string_type(ptr %arg1) {
 ; CHECK-SAME: stringLengthExp = <[DW_OP_push_object_address, DW_OP_plus_uconst(8)]>
 ; CHECK-SAME: stringLocationExp = <[DW_OP_push_object_address, DW_OP_deref]>>
 ; CHECK: #di_local_variable1 = #llvm.di_local_variable<scope = #di_subprogram, name = "str", file = #di_file, type = #di_string_type, flags = Artificial>
+
+; // -----
+
+; Test that imported entities for a functions are handled correctly.
+
+define void @imp_fn() !dbg !12 {
+  ret void
+}
+
+!llvm.module.flags = !{!10}
+!llvm.dbg.cu = !{!4}
+
+!2 = !DIModule(scope: !4, name: "mod1", file: !3, line: 1)
+!3 = !DIFile(filename: "test.f90", directory: "")
+!4 = distinct !DICompileUnit(language: DW_LANG_Fortran95, file: !3)
+!8 = !DIModule(scope: !4, name: "mod1", file: !3, line: 5)
+!10 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = distinct !DISubprogram(name: "imp_fn", linkageName: "imp_fn", scope: !3, file: !3, line: 10, type: !14, scopeLine: 10, spFlags: DISPFlagDefinition, unit: !4, retainedNodes: !16)
+!14 = !DISubroutineType(cc: DW_CC_program, types: !15)
+!15 = !{}
+!16 = !{!17}
+!17 = !DIImportedEntity(tag: DW_TAG_imported_module, scope: !12, entity: !8, file: !3, line: 1, elements: !15)
+
+; CHECK-DAG: #[[M:.+]] = #llvm.di_module<{{.*}}name = "mod1"{{.*}}>
+; CHECK-DAG: #[[SP:.+]] = #llvm.di_subprogram<{{.*}}name = "imp_fn"{{.*}}>
+; CHECK-DAG: llvm.di_imported_entity{{.*}}tag = DW_TAG_imported_module, scope = #[[SP]], entity = #[[M]]
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index 1a9a8561de00dc..2f9c53ff6da996 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -366,6 +366,39 @@ llvm.func @fn_with_gl() {
 
 // -----
 
+// Test that imported entries correctly generates 'retainedNodes' in the
+// subprogram.
+
+llvm.func @imp_fn() {
+  llvm.return
+} loc(#loc3)
+#file = #llvm.di_file<"test.f90" in "">
+#SP_TY = #llvm.di_subroutine_type<callingConvention = DW_CC_program>
+#CU = #llvm.di_compile_unit<id = distinct[0]<>,
+  sourceLanguage = DW_LANG_Fortran95, file = #file, isOptimized = false,
+  emissionKind = Full>
+#MOD = #llvm.di_module<file = #file, scope = #CU, name = "mod1">
+#MOD1 = #llvm.di_module<file = #file, scope = #CU, name = "mod2">
+#SP = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #CU, scope = #file,
+  name = "imp_fn", file = #file, subprogramFlags = Definition, type = #SP_TY>
+#ENT1 = #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #SP,
+  entity = #MOD1, file = #file>
+#ENT2 = #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #SP,
+  entity = #MOD, file = #file>
+#loc1 = loc("test.f90":12:14)
+#loc2 = loc(fused<#SP>[#loc1])
+#loc3 = loc(fused<[#ENT1, #ENT2]>[#loc2])
+
+
+// CHECK-DAG: ![[SP:[0-9]+]] = {{.*}}!DISubprogram(name: "imp_fn"{{.*}}retainedNodes: ![[NODES:[0-9]+]])
+// CHECK-DAG: ![[NODES]] = !{![[NODE2:[0-9]+]], ![[NODE1:[0-9]+]]}
+// CHECK-DAG: ![[NODE1]] = !DIImportedEntity(tag: DW_TAG_imported_module, scope: ![[SP]], entity: ![[MOD1:[0-9]+]]{{.*}})
+// CHECK-DAG: ![[NODE2]] = !DIImportedEntity(tag: DW_TAG_imported_module, scope: ![[SP]], entity: ![[MOD2:[0-9]+]]{{.*}})
+// CHECK-DAG: ![[MOD1]] = !DIModule({{.*}}name: "mod1"{{.*}})
+// CHECK-DAG: ![[MOD2]] = !DIModule({{.*}}name: "mod2"{{.*}})
+
+// -----
+
 // Nameless and scopeless global constant.
 
 // CHECK-LABEL: @.str.1 = external constant [10 x i8]

>From 1e16cc7f65d77393833628d1d4d18e628575e490 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 16 Aug 2024 16:08:59 +0100
Subject: [PATCH 2/5] [mlir][debug] Add retainedNodes field in
 DISubprogramAttr.

As discussed in the comments, this commit adds retainedNodes field in
the DISubprogramAttr. To remove the cyclic dependency, the scope
field has been removed from DIImportedEntity as it can be inferred
depending on the contained of the entities.
---
 mlir/include/mlir-c/Dialect/LLVM.h            |  9 +++-
 .../mlir/Dialect/LLVMIR/LLVMAttrDefs.td       | 11 +++--
 mlir/lib/CAPI/Dialect/LLVM.cpp                | 22 ++++++++-
 mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp      |  9 ++--
 .../Transforms/DIScopeForLLVMFuncOp.cpp       |  2 +-
 mlir/lib/Target/LLVMIR/DebugImporter.cpp      | 23 +++++----
 mlir/lib/Target/LLVMIR/DebugTranslation.cpp   | 48 ++++++++-----------
 mlir/lib/Target/LLVMIR/DebugTranslation.h     |  2 +-
 mlir/test/CAPI/llvm.c                         | 12 +++--
 mlir/test/Target/LLVMIR/Import/debug-info.ll  |  3 +-
 mlir/test/Target/LLVMIR/llvmir-debug.mlir     | 13 ++---
 11 files changed, 88 insertions(+), 66 deletions(-)

diff --git a/mlir/include/mlir-c/Dialect/LLVM.h b/mlir/include/mlir-c/Dialect/LLVM.h
index 631b5646183204..f09bf689213241 100644
--- a/mlir/include/mlir-c/Dialect/LLVM.h
+++ b/mlir/include/mlir-c/Dialect/LLVM.h
@@ -316,7 +316,8 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDISubprogramAttrGet(
     MlirContext ctx, MlirAttribute id, MlirAttribute compileUnit,
     MlirAttribute scope, MlirAttribute name, MlirAttribute linkageName,
     MlirAttribute file, unsigned int line, unsigned int scopeLine,
-    uint64_t subprogramFlags, MlirAttribute type);
+    uint64_t subprogramFlags, MlirAttribute type, intptr_t nNodes,
+    MlirAttribute const *nodes);
 
 /// Gets the scope from this DISubprogramAttr.
 MLIR_CAPI_EXPORTED MlirAttribute
@@ -353,6 +354,12 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDIModuleAttrGet(
     MlirAttribute name, MlirAttribute configMacros, MlirAttribute includePath,
     MlirAttribute apinotes, unsigned int line, bool isDecl);
 
+/// Creates a LLVM DIImportedEntityAttr attribute.
+MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDIImportedEntityAttrGet(
+    MlirContext ctx, unsigned int tag, MlirAttribute entity, MlirAttribute file,
+    unsigned int line, MlirAttribute name, intptr_t nNodes,
+    MlirAttribute const *elements);
+
 /// Gets the scope of this DIModuleAttr.
 MLIR_CAPI_EXPORTED MlirAttribute
 mlirLLVMDIModuleAttrGetScope(MlirAttribute diModule);
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index b9f99ef70cc884..4bc0d04c021d91 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -565,19 +565,21 @@ def LLVM_DISubprogramAttr : LLVM_Attr<"DISubprogram", "di_subprogram",
     OptionalParameter<"unsigned">:$line,
     OptionalParameter<"unsigned">:$scopeLine,
     OptionalParameter<"DISubprogramFlags">:$subprogramFlags,
-    OptionalParameter<"DISubroutineTypeAttr">:$type
+    OptionalParameter<"DISubroutineTypeAttr">:$type,
+    OptionalArrayRefParameter<"DINodeAttr">:$retainedNodes
   );
   let builders = [
     AttrBuilderWithInferredContext<(ins
       "DistinctAttr":$id, "DICompileUnitAttr":$compileUnit,
       "DIScopeAttr":$scope, "StringRef":$name, "StringRef":$linkageName,
       "DIFileAttr":$file, "unsigned":$line, "unsigned":$scopeLine,
-      "DISubprogramFlags":$subprogramFlags, "DISubroutineTypeAttr":$type
+      "DISubprogramFlags":$subprogramFlags, "DISubroutineTypeAttr":$type,
+      "ArrayRef<DINodeAttr>":$retainedNodes
     ), [{
       MLIRContext *ctx = file.getContext();
       return $_get(ctx, id, compileUnit, scope, StringAttr::get(ctx, name),
                    StringAttr::get(ctx, linkageName), file, line,
-                   scopeLine, subprogramFlags, type);
+                   scopeLine, subprogramFlags, type, retainedNodes);
     }]>
   ];
 
@@ -627,8 +629,7 @@ def LLVM_DIImportedEntityAttr : LLVM_Attr<"DIImportedEntity", "di_imported_entit
                                            /*traits=*/[], "DINodeAttr"> {
   let parameters = (ins
     LLVM_DITagParameter:$tag,
-    OptionalParameter<"DIScopeAttr">:$scope,
-    OptionalParameter<"DINodeAttr">:$entity,
+    "DINodeAttr":$entity,
     OptionalParameter<"DIFileAttr">:$file,
     OptionalParameter<"unsigned">:$line,
     OptionalParameter<"StringAttr">:$name,
diff --git a/mlir/lib/CAPI/Dialect/LLVM.cpp b/mlir/lib/CAPI/Dialect/LLVM.cpp
index 03e2f2be2156af..edd21bb97fc63c 100644
--- a/mlir/lib/CAPI/Dialect/LLVM.cpp
+++ b/mlir/lib/CAPI/Dialect/LLVM.cpp
@@ -293,14 +293,19 @@ MlirAttribute mlirLLVMDISubprogramAttrGet(
     MlirContext ctx, MlirAttribute id, MlirAttribute compileUnit,
     MlirAttribute scope, MlirAttribute name, MlirAttribute linkageName,
     MlirAttribute file, unsigned int line, unsigned int scopeLine,
-    uint64_t subprogramFlags, MlirAttribute type) {
+    uint64_t subprogramFlags, MlirAttribute type, intptr_t nNodes,
+    MlirAttribute const *nodes) {
+  SmallVector<Attribute> nodesStorage;
+  nodesStorage.reserve(nNodes);
   return wrap(DISubprogramAttr::get(
       unwrap(ctx), cast<DistinctAttr>(unwrap(id)),
       cast<DICompileUnitAttr>(unwrap(compileUnit)),
       cast<DIScopeAttr>(unwrap(scope)), cast<StringAttr>(unwrap(name)),
       cast<StringAttr>(unwrap(linkageName)), cast<DIFileAttr>(unwrap(file)),
       line, scopeLine, DISubprogramFlags(subprogramFlags),
-      cast<DISubroutineTypeAttr>(unwrap(type))));
+      cast<DISubroutineTypeAttr>(unwrap(type)),
+      llvm::map_to_vector(unwrapList(nNodes, nodes, nodesStorage),
+                          [](Attribute a) { return cast<DINodeAttr>(a); })));
 }
 
 MlirAttribute mlirLLVMDISubprogramAttrGetScope(MlirAttribute diSubprogram) {
@@ -345,3 +350,16 @@ MlirAttribute mlirLLVMDIModuleAttrGet(MlirContext ctx, MlirAttribute file,
 MlirAttribute mlirLLVMDIModuleAttrGetScope(MlirAttribute diModule) {
   return wrap(cast<DIModuleAttr>(unwrap(diModule)).getScope());
 }
+
+MlirAttribute mlirLLVMDIImportedEntityAttrGet(
+    MlirContext ctx, unsigned int tag, MlirAttribute entity, MlirAttribute file,
+    unsigned int line, MlirAttribute name, intptr_t nElements,
+    MlirAttribute const *elements) {
+  SmallVector<Attribute> elementsStorage;
+  elementsStorage.reserve(nElements);
+  return wrap(DIImportedEntityAttr::get(
+      unwrap(ctx), tag, cast<DINodeAttr>(unwrap(entity)),
+      cast<DIFileAttr>(unwrap(file)), line, cast<StringAttr>(unwrap(name)),
+      llvm::map_to_vector(unwrapList(nElements, elements, elementsStorage),
+                          [](Attribute a) { return cast<DINodeAttr>(a); })));
+}
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index 963a4be25079e3..98a9659735e7e6 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -58,10 +58,11 @@ void LLVMDialect::registerAttributes() {
 bool DINodeAttr::classof(Attribute attr) {
   return llvm::isa<DIBasicTypeAttr, DICompileUnitAttr, DICompositeTypeAttr,
                    DIDerivedTypeAttr, DIFileAttr, DIGlobalVariableAttr,
-                   DILabelAttr, DILexicalBlockAttr, DILexicalBlockFileAttr,
-                   DILocalVariableAttr, DIModuleAttr, DINamespaceAttr,
-                   DINullTypeAttr, DIStringTypeAttr, DISubprogramAttr,
-                   DISubrangeAttr, DISubroutineTypeAttr>(attr);
+                   DIImportedEntityAttr, DILabelAttr, DILexicalBlockAttr,
+                   DILexicalBlockFileAttr, DILocalVariableAttr, DIModuleAttr,
+                   DINamespaceAttr, DINullTypeAttr, DIStringTypeAttr,
+                   DISubprogramAttr, DISubrangeAttr, DISubroutineTypeAttr>(
+      attr);
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
index 395ff6ed1e48ea..eecf504a8b901b 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
@@ -79,7 +79,7 @@ static void addScopeToFunction(LLVM::LLVMFuncOp llvmFunc,
       context, id, compileUnitAttr, fileAttr, funcNameAttr, funcNameAttr,
       fileAttr,
       /*line=*/line,
-      /*scopeline=*/col, subprogramFlags, subroutineTypeAttr);
+      /*scopeline=*/col, subprogramFlags, subroutineTypeAttr, {});
   llvmFunc->setLoc(FusedLoc::get(context, {loc}, subprogramAttr));
 }
 
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
index 77fd292a5cca12..36b03493437a2e 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -40,18 +40,10 @@ Location DebugImporter::translateFuncLocation(llvm::Function *func) {
   // Add a fused location to link the subprogram information.
   StringAttr funcName = StringAttr::get(context, subprogram->getName());
   StringAttr fileName = StringAttr::get(context, subprogram->getFilename());
-  auto loc = FusedLocWith<DISubprogramAttr>::get(
+  return FusedLocWith<DISubprogramAttr>::get(
       {NameLoc::get(funcName),
        FileLineColLoc::get(fileName, subprogram->getLine(), /*column=*/0)},
       translate(subprogram), context);
-  if (subprogram->getRetainedNodes().empty())
-    return loc;
-  llvm::SmallVector<mlir::Attribute> entities;
-  for (auto node : subprogram->getRetainedNodes())
-    entities.push_back(translate(node));
-
-  auto entitiesAttr = mlir::ArrayAttr::get(context, entities);
-  return FusedLocWith<mlir::ArrayAttr>::get(loc, entitiesAttr, context);
 }
 
 //===----------------------------------------------------------------------===//
@@ -226,8 +218,8 @@ DebugImporter::translateImpl(llvm::DIImportedEntity *node) {
   }
 
   return DIImportedEntityAttr::get(
-      context, node->getTag(), translate(node->getScope()),
-      translate(node->getEntity()), translate(node->getFile()), node->getLine(),
+      context, node->getTag(), translate(node->getEntity()),
+      translate(node->getFile()), node->getLine(),
       getStringAttrOrNull(node->getRawName()), elements);
 }
 
@@ -246,11 +238,18 @@ DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
   DISubroutineTypeAttr type = translate(node->getType());
   if (node->getType() && !type)
     return nullptr;
+
+  SmallVector<DINodeAttr> retainedNodes;
+
+  for (auto node : node->getRetainedNodes())
+    retainedNodes.push_back(translate(node));
+
   return DISubprogramAttr::get(context, id, translate(node->getUnit()), scope,
                                getStringAttrOrNull(node->getRawName()),
                                getStringAttrOrNull(node->getRawLinkageName()),
                                translate(node->getFile()), node->getLine(),
-                               node->getScopeLine(), *subprogramFlags, type);
+                               node->getScopeLine(), *subprogramFlags, type,
+                               retainedNodes);
 }
 
 DISubrangeAttr DebugImporter::translateImpl(llvm::DISubrange *node) {
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index 93ca3cfc5eba72..6acdb13c3637f6 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -67,25 +67,7 @@ void DebugTranslation::translate(LLVMFuncOp func, llvm::Function &llvmFunc) {
       func.getLoc()->findInstanceOf<FusedLocWith<LLVM::DISubprogramAttr>>();
   if (!spLoc)
     return;
-  llvm::DISubprogram *sp = translate(spLoc.getMetadata());
-  llvmFunc.setSubprogram(sp);
-
-  // Look for any entities that are imported in this function.
-  auto entityLoc =
-      func.getLoc()->findInstanceOf<FusedLocWith<mlir::ArrayAttr>>();
-  if (!entityLoc)
-    return;
-
-  SmallVector<llvm::Metadata *> imported;
-  if (mlir::ArrayAttr arrayAttr = entityLoc.getMetadata()) {
-    for (mlir::Attribute attr : arrayAttr.getValue()) {
-      if (auto ent = dyn_cast_if_present<LLVM::DIImportedEntityAttr>(attr)) {
-        llvm::DINode *node = translate(ent);
-        imported.push_back(node);
-      }
-    }
-  }
-  sp->replaceRetainedNodes(llvm::MDTuple::get(llvmFunc.getContext(), imported));
+  llvmFunc.setSubprogram(translate(spLoc.getMetadata()));
 }
 
 //===----------------------------------------------------------------------===//
@@ -324,6 +306,18 @@ llvm::DISubprogram *DebugTranslation::translateImpl(DISubprogramAttr attr) {
       static_cast<llvm::DISubprogram::DISPFlags>(attr.getSubprogramFlags()),
       compileUnit);
 
+  SmallVector<llvm::Metadata *> retainedNodes;
+
+  for (auto nodeAttr : attr.getRetainedNodes()) {
+    if (DIImportedEntityAttr importedAttr =
+            dyn_cast<DIImportedEntityAttr>(nodeAttr)) {
+      llvm::DINode *dn = translate(importedAttr, node);
+      retainedNodes.push_back(dn);
+    }
+  }
+  if (!retainedNodes.empty())
+    node->replaceRetainedNodes(llvm::MDTuple::get(llvmCtx, retainedNodes));
+
   if (attr.getId())
     distinctAttrToNode.try_emplace(attr.getId(), node);
   return node;
@@ -344,15 +338,15 @@ llvm::DINamespace *DebugTranslation::translateImpl(DINamespaceAttr attr) {
                                 attr.getExportSymbols());
 }
 
-llvm::DIImportedEntity *
-DebugTranslation::translateImpl(DIImportedEntityAttr attr) {
+llvm::DIImportedEntity *DebugTranslation::translate(DIImportedEntityAttr attr,
+                                                    llvm::DIScope *scope) {
   SmallVector<llvm::Metadata *> elements;
   for (DINodeAttr member : attr.getElements())
     elements.push_back(translate(member));
 
   return llvm::DIImportedEntity::get(
-      llvmCtx, attr.getTag(), translate(attr.getScope()),
-      translate(attr.getEntity()), translate(attr.getFile()), attr.getLine(),
+      llvmCtx, attr.getTag(), scope, translate(attr.getEntity()),
+      translate(attr.getFile()), attr.getLine(),
       getMDStringOrNull(attr.getName()), llvm::MDNode::get(llvmCtx, elements));
 }
 
@@ -418,10 +412,10 @@ llvm::DINode *DebugTranslation::translate(DINodeAttr attr) {
     node = TypeSwitch<DINodeAttr, llvm::DINode *>(attr)
                .Case<DIBasicTypeAttr, DICompileUnitAttr, DICompositeTypeAttr,
                      DIDerivedTypeAttr, DIFileAttr, DIGlobalVariableAttr,
-                     DIImportedEntityAttr, DILabelAttr, DILexicalBlockAttr,
-                     DILexicalBlockFileAttr, DILocalVariableAttr, DIModuleAttr,
-                     DINamespaceAttr, DINullTypeAttr, DIStringTypeAttr,
-                     DISubprogramAttr, DISubrangeAttr, DISubroutineTypeAttr>(
+                     DILabelAttr, DILexicalBlockAttr, DILexicalBlockFileAttr,
+                     DILocalVariableAttr, DIModuleAttr, DINamespaceAttr,
+                     DINullTypeAttr, DIStringTypeAttr, DISubprogramAttr,
+                     DISubrangeAttr, DISubroutineTypeAttr>(
                    [&](auto attr) { return translateImpl(attr); });
 
   if (node && !node->isTemporary())
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.h b/mlir/lib/Target/LLVMIR/DebugTranslation.h
index 3f28297fc7f594..7581177fc8a347 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.h
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.h
@@ -84,12 +84,12 @@ class DebugTranslation {
   llvm::DIVariable *translateImpl(DIVariableAttr attr);
   llvm::DIModule *translateImpl(DIModuleAttr attr);
   llvm::DINamespace *translateImpl(DINamespaceAttr attr);
-  llvm::DIImportedEntity *translateImpl(DIImportedEntityAttr attr);
   llvm::DIScope *translateImpl(DIScopeAttr attr);
   llvm::DISubprogram *translateImpl(DISubprogramAttr attr);
   llvm::DISubrange *translateImpl(DISubrangeAttr attr);
   llvm::DISubroutineType *translateImpl(DISubroutineTypeAttr attr);
   llvm::DIType *translateImpl(DITypeAttr attr);
+  llvm::DIImportedEntity *translate(DIImportedEntityAttr attr, llvm::DIScope *);
 
   /// Attributes that support self recursion need to implement an additional
   /// method to hook into `translateRecursive`.
diff --git a/mlir/test/CAPI/llvm.c b/mlir/test/CAPI/llvm.c
index d3054aa6a0d93d..da28a96f89691d 100644
--- a/mlir/test/CAPI/llvm.c
+++ b/mlir/test/CAPI/llvm.c
@@ -312,9 +312,15 @@ static void testDebugInfoAttributes(MlirContext ctx) {
   // CHECK: #llvm.di_subroutine_type<{{.*}}>
   mlirAttributeDump(subroutine_type);
 
-  MlirAttribute di_subprogram =
-      mlirLLVMDISubprogramAttrGet(ctx, id, compile_unit, compile_unit, foo, bar,
-                                  file, 1, 2, 0, subroutine_type);
+  MlirAttribute di_imported_entity = mlirLLVMDIImportedEntityAttrGet(
+      ctx, 0, di_module, file, 1, foo, 1, &local_var);
+
+  mlirAttributeDump(di_imported_entity);
+  // CHECK: #llvm.di_imported_entity<{{.*}}>
+
+  MlirAttribute di_subprogram = mlirLLVMDISubprogramAttrGet(
+      ctx, id, compile_unit, compile_unit, foo, bar, file, 1, 2, 0,
+      subroutine_type, 1, &di_imported_entity);
   // CHECK: #llvm.di_subprogram<{{.*}}>
   mlirAttributeDump(di_subprogram);
 
diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll
index 77ed6183cebae4..bb03da37c0d097 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -816,5 +816,4 @@ define void @imp_fn() !dbg !12 {
 !17 = !DIImportedEntity(tag: DW_TAG_imported_module, scope: !12, entity: !8, file: !3, line: 1, elements: !15)
 
 ; CHECK-DAG: #[[M:.+]] = #llvm.di_module<{{.*}}name = "mod1"{{.*}}>
-; CHECK-DAG: #[[SP:.+]] = #llvm.di_subprogram<{{.*}}name = "imp_fn"{{.*}}>
-; CHECK-DAG: llvm.di_imported_entity{{.*}}tag = DW_TAG_imported_module, scope = #[[SP]], entity = #[[M]]
+; CHECK-DAG: #[[SP:.+]] = #llvm.di_subprogram<{{.*}}name = "imp_fn"{{.*}}retainedNodes = #llvm.di_imported_entity<tag = DW_TAG_imported_module, entity = #[[M]]{{.*}}>>
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index 2f9c53ff6da996..30b2ba5e9bad1f 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -371,7 +371,7 @@ llvm.func @fn_with_gl() {
 
 llvm.func @imp_fn() {
   llvm.return
-} loc(#loc3)
+} loc(#loc2)
 #file = #llvm.di_file<"test.f90" in "">
 #SP_TY = #llvm.di_subroutine_type<callingConvention = DW_CC_program>
 #CU = #llvm.di_compile_unit<id = distinct[0]<>,
@@ -380,15 +380,12 @@ llvm.func @imp_fn() {
 #MOD = #llvm.di_module<file = #file, scope = #CU, name = "mod1">
 #MOD1 = #llvm.di_module<file = #file, scope = #CU, name = "mod2">
 #SP = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #CU, scope = #file,
-  name = "imp_fn", file = #file, subprogramFlags = Definition, type = #SP_TY>
-#ENT1 = #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #SP,
-  entity = #MOD1, file = #file>
-#ENT2 = #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #SP,
-  entity = #MOD, file = #file>
+  name = "imp_fn", file = #file, subprogramFlags = Definition, type = #SP_TY,
+  retainedNodes = #llvm.di_imported_entity<tag = DW_TAG_imported_module,
+  entity = #MOD1, file = #file, line = 1>, #llvm.di_imported_entity<tag
+  = DW_TAG_imported_module, entity = #MOD, file = #file, line = 1>>
 #loc1 = loc("test.f90":12:14)
 #loc2 = loc(fused<#SP>[#loc1])
-#loc3 = loc(fused<[#ENT1, #ENT2]>[#loc2])
-
 
 // CHECK-DAG: ![[SP:[0-9]+]] = {{.*}}!DISubprogram(name: "imp_fn"{{.*}}retainedNodes: ![[NODES:[0-9]+]])
 // CHECK-DAG: ![[NODES]] = !{![[NODE2:[0-9]+]], ![[NODE1:[0-9]+]]}

>From 895f3c0c2c57ef4decab49ff4530ffa23aeb7a59 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 20 Aug 2024 12:28:34 +0100
Subject: [PATCH 3/5] Handle review comments.

Mostly formatting nits and adding comments.
---
 mlir/include/mlir-c/Dialect/LLVM.h                  |  4 ++--
 mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td    |  5 +++++
 mlir/lib/CAPI/Dialect/LLVM.cpp                      | 13 +++++++------
 .../LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp      |  3 ++-
 mlir/lib/Target/LLVMIR/DebugImporter.cpp            |  6 ++----
 mlir/lib/Target/LLVMIR/DebugTranslation.cpp         |  9 +++++----
 mlir/lib/Target/LLVMIR/DebugTranslation.h           |  5 +++++
 7 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/mlir/include/mlir-c/Dialect/LLVM.h b/mlir/include/mlir-c/Dialect/LLVM.h
index f09bf689213241..72741331d6c7ee 100644
--- a/mlir/include/mlir-c/Dialect/LLVM.h
+++ b/mlir/include/mlir-c/Dialect/LLVM.h
@@ -357,8 +357,8 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDIModuleAttrGet(
 /// Creates a LLVM DIImportedEntityAttr attribute.
 MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDIImportedEntityAttrGet(
     MlirContext ctx, unsigned int tag, MlirAttribute entity, MlirAttribute file,
-    unsigned int line, MlirAttribute name, intptr_t nNodes,
-    MlirAttribute const *elements);
+    unsigned int line, MlirAttribute name, intptr_t nRetainedNodes,
+    MlirAttribute const *retainedNodes);
 
 /// Gets the scope of this DIModuleAttr.
 MLIR_CAPI_EXPORTED MlirAttribute
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 4bc0d04c021d91..e57be7f760d380 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -627,6 +627,11 @@ def LLVM_DINamespaceAttr : LLVM_Attr<"DINamespace", "di_namespace",
 
 def LLVM_DIImportedEntityAttr : LLVM_Attr<"DIImportedEntity", "di_imported_entity",
                                            /*traits=*/[], "DINodeAttr"> {
+  /// TODO: DIImportedEntity has a 'scope' field which represents the scope where
+  /// this entity is imported. Currently, we are not adding a 'scope' field in
+  /// DIImportedEntityAttr to avoid cyclic dependency. As DIImportedEntityAttr
+  /// entries will be contained inside a scope entity (e.g. DISubprogramAttr),
+  /// the scope can easily be inferred.
   let parameters = (ins
     LLVM_DITagParameter:$tag,
     "DINodeAttr":$entity,
diff --git a/mlir/lib/CAPI/Dialect/LLVM.cpp b/mlir/lib/CAPI/Dialect/LLVM.cpp
index edd21bb97fc63c..bc1b8f38c5bdf0 100644
--- a/mlir/lib/CAPI/Dialect/LLVM.cpp
+++ b/mlir/lib/CAPI/Dialect/LLVM.cpp
@@ -353,13 +353,14 @@ MlirAttribute mlirLLVMDIModuleAttrGetScope(MlirAttribute diModule) {
 
 MlirAttribute mlirLLVMDIImportedEntityAttrGet(
     MlirContext ctx, unsigned int tag, MlirAttribute entity, MlirAttribute file,
-    unsigned int line, MlirAttribute name, intptr_t nElements,
-    MlirAttribute const *elements) {
-  SmallVector<Attribute> elementsStorage;
-  elementsStorage.reserve(nElements);
+    unsigned int line, MlirAttribute name, intptr_t nRetainedNodes,
+    MlirAttribute const *retainedNodes) {
+  SmallVector<Attribute> nodesStorage;
+  nodesStorage.reserve(nRetainedNodes);
   return wrap(DIImportedEntityAttr::get(
       unwrap(ctx), tag, cast<DINodeAttr>(unwrap(entity)),
       cast<DIFileAttr>(unwrap(file)), line, cast<StringAttr>(unwrap(name)),
-      llvm::map_to_vector(unwrapList(nElements, elements, elementsStorage),
-                          [](Attribute a) { return cast<DINodeAttr>(a); })));
+      llvm::map_to_vector(
+          unwrapList(nRetainedNodes, retainedNodes, nodesStorage),
+          [](Attribute a) { return cast<DINodeAttr>(a); })));
 }
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
index eecf504a8b901b..758700c9272bc8 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
@@ -79,7 +79,8 @@ static void addScopeToFunction(LLVM::LLVMFuncOp llvmFunc,
       context, id, compileUnitAttr, fileAttr, funcNameAttr, funcNameAttr,
       fileAttr,
       /*line=*/line,
-      /*scopeline=*/col, subprogramFlags, subroutineTypeAttr, {});
+      /*scopeline=*/col, subprogramFlags, subroutineTypeAttr,
+      /*retainedNodes=*/{});
   llvmFunc->setLoc(FusedLoc::get(context, {loc}, subprogramAttr));
 }
 
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
index 36b03493437a2e..ce3643f513d34a 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -211,7 +211,6 @@ DINamespaceAttr DebugImporter::translateImpl(llvm::DINamespace *node) {
 DIImportedEntityAttr
 DebugImporter::translateImpl(llvm::DIImportedEntity *node) {
   SmallVector<DINodeAttr> elements;
-
   for (llvm::DINode *element : node->getElements()) {
     assert(element && "expected a non-null element type");
     elements.push_back(translate(element));
@@ -240,9 +239,8 @@ DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
     return nullptr;
 
   SmallVector<DINodeAttr> retainedNodes;
-
-  for (auto node : node->getRetainedNodes())
-    retainedNodes.push_back(translate(node));
+  for (llvm::DINode *retainedNode : node->getRetainedNodes())
+    retainedNodes.push_back(translate(retainedNode));
 
   return DISubprogramAttr::get(context, id, translate(node->getUnit()), scope,
                                getStringAttrOrNull(node->getRawName()),
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index 6acdb13c3637f6..042e015f107fea 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -306,11 +306,12 @@ llvm::DISubprogram *DebugTranslation::translateImpl(DISubprogramAttr attr) {
       static_cast<llvm::DISubprogram::DISPFlags>(attr.getSubprogramFlags()),
       compileUnit);
 
+  // DIImportedEntity requires scope information which DIImportedEntityAttr does
+  // not have. This is why we translate DIImportedEntityAttr after we have
+  // created DISubprogram as we can use it as the scope.
   SmallVector<llvm::Metadata *> retainedNodes;
-
-  for (auto nodeAttr : attr.getRetainedNodes()) {
-    if (DIImportedEntityAttr importedAttr =
-            dyn_cast<DIImportedEntityAttr>(nodeAttr)) {
+  for (DINodeAttr nodeAttr : attr.getRetainedNodes()) {
+    if (auto importedAttr = dyn_cast<DIImportedEntityAttr>(nodeAttr)) {
       llvm::DINode *dn = translate(importedAttr, node);
       retainedNodes.push_back(dn);
     }
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.h b/mlir/lib/Target/LLVMIR/DebugTranslation.h
index 7581177fc8a347..37b985acf8541e 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.h
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.h
@@ -89,6 +89,11 @@ class DebugTranslation {
   llvm::DISubrange *translateImpl(DISubrangeAttr attr);
   llvm::DISubroutineType *translateImpl(DISubroutineTypeAttr attr);
   llvm::DIType *translateImpl(DITypeAttr attr);
+
+  /// Currently, DIImportedEntityAttr does not have a scope field to avoid a
+  /// cyclic dependency.  The scope information is obtained from the entity
+  /// which holds the list of DIImportedEntityAttr. This requires that scope
+  /// information be passed to translate function.
   llvm::DIImportedEntity *translate(DIImportedEntityAttr attr, llvm::DIScope *);
 
   /// Attributes that support self recursion need to implement an additional

>From 957420db7a8cef377280cafc95c65b4ec3f1aab0 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 20 Aug 2024 12:43:08 +0100
Subject: [PATCH 4/5] Fix flang build.

Add a default value for retainedNodes field.
---
 flang/lib/Optimizer/Transforms/AddDebugInfo.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 30fc4185575e61..286af418fc05dd 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -302,7 +302,7 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
 
   auto spAttr = mlir::LLVM::DISubprogramAttr::get(
       context, id, compilationUnit, Scope, funcName, fullName, funcFileAttr,
-      line, line, subprogramFlags, subTypeAttr);
+      line, line, subprogramFlags, subTypeAttr, /*retainedNodes=*/{});
   funcOp->setLoc(builder.getFusedLoc({funcOp->getLoc()}, spAttr));
 
   // Don't process variables if user asked for line tables only.

>From aea831144076b5533eeaf68b89415ebb0d1f79fe Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 23 Aug 2024 10:01:40 +0100
Subject: [PATCH 5/5] Fix some typos.

The function parameter names were swapped in 2 places.
---
 mlir/include/mlir-c/Dialect/LLVM.h |  8 ++++----
 mlir/lib/CAPI/Dialect/LLVM.cpp     | 24 ++++++++++++------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/mlir/include/mlir-c/Dialect/LLVM.h b/mlir/include/mlir-c/Dialect/LLVM.h
index 72741331d6c7ee..5eb96a86e472d6 100644
--- a/mlir/include/mlir-c/Dialect/LLVM.h
+++ b/mlir/include/mlir-c/Dialect/LLVM.h
@@ -316,8 +316,8 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDISubprogramAttrGet(
     MlirContext ctx, MlirAttribute id, MlirAttribute compileUnit,
     MlirAttribute scope, MlirAttribute name, MlirAttribute linkageName,
     MlirAttribute file, unsigned int line, unsigned int scopeLine,
-    uint64_t subprogramFlags, MlirAttribute type, intptr_t nNodes,
-    MlirAttribute const *nodes);
+    uint64_t subprogramFlags, MlirAttribute type, intptr_t nRetainedNodes,
+    MlirAttribute const *retainedNodes);
 
 /// Gets the scope from this DISubprogramAttr.
 MLIR_CAPI_EXPORTED MlirAttribute
@@ -357,8 +357,8 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDIModuleAttrGet(
 /// Creates a LLVM DIImportedEntityAttr attribute.
 MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDIImportedEntityAttrGet(
     MlirContext ctx, unsigned int tag, MlirAttribute entity, MlirAttribute file,
-    unsigned int line, MlirAttribute name, intptr_t nRetainedNodes,
-    MlirAttribute const *retainedNodes);
+    unsigned int line, MlirAttribute name, intptr_t nElements,
+    MlirAttribute const *elements);
 
 /// Gets the scope of this DIModuleAttr.
 MLIR_CAPI_EXPORTED MlirAttribute
diff --git a/mlir/lib/CAPI/Dialect/LLVM.cpp b/mlir/lib/CAPI/Dialect/LLVM.cpp
index bc1b8f38c5bdf0..13341f0c4de881 100644
--- a/mlir/lib/CAPI/Dialect/LLVM.cpp
+++ b/mlir/lib/CAPI/Dialect/LLVM.cpp
@@ -293,10 +293,10 @@ MlirAttribute mlirLLVMDISubprogramAttrGet(
     MlirContext ctx, MlirAttribute id, MlirAttribute compileUnit,
     MlirAttribute scope, MlirAttribute name, MlirAttribute linkageName,
     MlirAttribute file, unsigned int line, unsigned int scopeLine,
-    uint64_t subprogramFlags, MlirAttribute type, intptr_t nNodes,
-    MlirAttribute const *nodes) {
+    uint64_t subprogramFlags, MlirAttribute type, intptr_t nRetainedNodes,
+    MlirAttribute const *retainedNodes) {
   SmallVector<Attribute> nodesStorage;
-  nodesStorage.reserve(nNodes);
+  nodesStorage.reserve(nRetainedNodes);
   return wrap(DISubprogramAttr::get(
       unwrap(ctx), cast<DistinctAttr>(unwrap(id)),
       cast<DICompileUnitAttr>(unwrap(compileUnit)),
@@ -304,8 +304,9 @@ MlirAttribute mlirLLVMDISubprogramAttrGet(
       cast<StringAttr>(unwrap(linkageName)), cast<DIFileAttr>(unwrap(file)),
       line, scopeLine, DISubprogramFlags(subprogramFlags),
       cast<DISubroutineTypeAttr>(unwrap(type)),
-      llvm::map_to_vector(unwrapList(nNodes, nodes, nodesStorage),
-                          [](Attribute a) { return cast<DINodeAttr>(a); })));
+      llvm::map_to_vector(
+          unwrapList(nRetainedNodes, retainedNodes, nodesStorage),
+          [](Attribute a) { return cast<DINodeAttr>(a); })));
 }
 
 MlirAttribute mlirLLVMDISubprogramAttrGetScope(MlirAttribute diSubprogram) {
@@ -353,14 +354,13 @@ MlirAttribute mlirLLVMDIModuleAttrGetScope(MlirAttribute diModule) {
 
 MlirAttribute mlirLLVMDIImportedEntityAttrGet(
     MlirContext ctx, unsigned int tag, MlirAttribute entity, MlirAttribute file,
-    unsigned int line, MlirAttribute name, intptr_t nRetainedNodes,
-    MlirAttribute const *retainedNodes) {
-  SmallVector<Attribute> nodesStorage;
-  nodesStorage.reserve(nRetainedNodes);
+    unsigned int line, MlirAttribute name, intptr_t nElements,
+    MlirAttribute const *elements) {
+  SmallVector<Attribute> elementsStorage;
+  elementsStorage.reserve(nElements);
   return wrap(DIImportedEntityAttr::get(
       unwrap(ctx), tag, cast<DINodeAttr>(unwrap(entity)),
       cast<DIFileAttr>(unwrap(file)), line, cast<StringAttr>(unwrap(name)),
-      llvm::map_to_vector(
-          unwrapList(nRetainedNodes, retainedNodes, nodesStorage),
-          [](Attribute a) { return cast<DINodeAttr>(a); })));
+      llvm::map_to_vector(unwrapList(nElements, elements, elementsStorage),
+                          [](Attribute a) { return cast<DINodeAttr>(a); })));
 }



More information about the flang-commits mailing list