[Mlir-commits] [mlir] [MLIR][LLVM] Make DISubprogramAttr cyclic (PR #106571)

Tobias Gysi llvmlistbot at llvm.org
Thu Aug 29 09:01:56 PDT 2024


https://github.com/gysit updated https://github.com/llvm/llvm-project/pull/106571

>From 9274e390ab40cc22a0984ad8ecd53ef5e8bbc2be Mon Sep 17 00:00:00 2001
From: Tobias Gysi <tobias.gysi at nextsilicon.com>
Date: Thu, 29 Aug 2024 15:29:10 +0000
Subject: [PATCH] [MLIR][LLVM] Make DISubprogramAttr cyclic

This commit implements LLVM_DIRecursiveTypeAttrInterface for the
DISubprogramAttr to ensure cyclic subprograms can be imported properly.
In the process multiple shortcuts around the recently introduced
DIImportedEntityAttr can be removed.
---
 mlir/include/mlir-c/Dialect/LLVM.h            | 22 +++--
 .../mlir/Dialect/LLVMIR/LLVMAttrDefs.td       | 33 +++++--
 mlir/lib/CAPI/Dialect/LLVM.cpp                | 31 ++++--
 mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp      | 16 +++
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp    |  6 +-
 .../Transforms/DIScopeForLLVMFuncOp.cpp       |  4 +-
 mlir/lib/Target/LLVMIR/DebugImporter.cpp      | 23 +++--
 mlir/lib/Target/LLVMIR/DebugTranslation.cpp   | 98 ++++++++++---------
 mlir/lib/Target/LLVMIR/DebugTranslation.h     | 18 ++--
 mlir/test/CAPI/llvm.c                         | 19 +++-
 mlir/test/Target/LLVMIR/Import/debug-info.ll  | 20 ++--
 mlir/test/Target/LLVMIR/llvmir-debug.mlir     | 35 ++++---
 12 files changed, 200 insertions(+), 125 deletions(-)

diff --git a/mlir/include/mlir-c/Dialect/LLVM.h b/mlir/include/mlir-c/Dialect/LLVM.h
index 5eb96a86e472d6..561d698f722afe 100644
--- a/mlir/include/mlir-c/Dialect/LLVM.h
+++ b/mlir/include/mlir-c/Dialect/LLVM.h
@@ -234,6 +234,9 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDIBasicTypeAttrGet(
     MlirContext ctx, unsigned int tag, MlirAttribute name, uint64_t sizeInBits,
     MlirLLVMTypeEncoding encoding);
 
+/// Creates a self-referencing LLVM DICompositeType attribute.
+MlirAttribute mlirLLVMDICompositeTypeAttrGetSelfRec(MlirAttribute recId);
+
 /// Creates a LLVM DICompositeType attribute.
 MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDICompositeTypeAttrGet(
     MlirContext ctx, unsigned int tag, MlirAttribute recId, MlirAttribute name,
@@ -311,13 +314,16 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDILocalVariableAttrGet(
     MlirAttribute diFile, unsigned int line, unsigned int arg,
     unsigned int alignInBits, MlirAttribute diType, int64_t flags);
 
+/// Creates a self-referencing LLVM DISubprogramAttr attribute.
+MlirAttribute mlirLLVMDISubprogramAttrGetSelfRec(MlirAttribute recId);
+
 /// Creates a LLVM DISubprogramAttr attribute.
 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 nRetainedNodes,
-    MlirAttribute const *retainedNodes);
+    MlirContext ctx, MlirAttribute id, MlirAttribute recId,
+    MlirAttribute compileUnit, MlirAttribute scope, MlirAttribute name,
+    MlirAttribute linkageName, MlirAttribute file, unsigned int line,
+    unsigned int scopeLine, uint64_t subprogramFlags, MlirAttribute type,
+    intptr_t nRetainedNodes, MlirAttribute const *retainedNodes);
 
 /// Gets the scope from this DISubprogramAttr.
 MLIR_CAPI_EXPORTED MlirAttribute
@@ -356,9 +362,9 @@ 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 nElements,
-    MlirAttribute const *elements);
+    MlirContext ctx, unsigned int tag, MlirAttribute scope,
+    MlirAttribute entity, MlirAttribute file, 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/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index e57be7f760d380..224e58f3216abf 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -554,14 +554,16 @@ def LLVM_DILocalVariableAttr : LLVM_Attr<"DILocalVariable", "di_local_variable",
 //===----------------------------------------------------------------------===//
 
 def LLVM_DISubprogramAttr : LLVM_Attr<"DISubprogram", "di_subprogram",
-                                      /*traits=*/[], "DIScopeAttr"> {
+                                      [LLVM_DIRecursiveTypeAttrInterface],
+                                      "DIScopeAttr"> {
   let parameters = (ins
     OptionalParameter<"DistinctAttr">:$id,
+    OptionalParameter<"DistinctAttr">:$recId,
     OptionalParameter<"DICompileUnitAttr">:$compileUnit,
-    "DIScopeAttr":$scope,
+    OptionalParameter<"DIScopeAttr">:$scope,
     OptionalParameter<"StringAttr">:$name,
     OptionalParameter<"StringAttr">:$linkageName,
-    "DIFileAttr":$file,
+    OptionalParameter<"DIFileAttr">:$file,
     OptionalParameter<"unsigned">:$line,
     OptionalParameter<"unsigned">:$scopeLine,
     OptionalParameter<"DISubprogramFlags">:$subprogramFlags,
@@ -577,13 +579,28 @@ def LLVM_DISubprogramAttr : LLVM_Attr<"DISubprogram", "di_subprogram",
       "ArrayRef<DINodeAttr>":$retainedNodes
     ), [{
       MLIRContext *ctx = file.getContext();
-      return $_get(ctx, id, compileUnit, scope, StringAttr::get(ctx, name),
+      return $_get(ctx, id, /*recId=*/nullptr, compileUnit, scope,
+                   StringAttr::get(ctx, name),
                    StringAttr::get(ctx, linkageName), file, line,
                    scopeLine, subprogramFlags, type, retainedNodes);
     }]>
   ];
-
   let assemblyFormat = "`<` struct(params) `>`";
+  let extraClassDeclaration = [{
+    /// Requirements of DIRecursiveTypeAttrInterface.
+    /// @{
+
+    /// Get whether this attr describes a recursive self reference.
+    bool isRecSelf() { return !getScope(); }
+
+    /// Get a copy of this type attr but with the recursive ID set to `recId`.
+    DIRecursiveTypeAttrInterface withRecId(DistinctAttr recId);
+
+    /// Build a rec-self instance using the provided `recId`.
+    static DIRecursiveTypeAttrInterface getRecSelf(DistinctAttr recId);
+
+    /// @}
+  }];
 }
 
 //===----------------------------------------------------------------------===//
@@ -627,13 +644,9 @@ 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,
+    "DIScopeAttr":$scope,
     "DINodeAttr":$entity,
     OptionalParameter<"DIFileAttr">:$file,
     OptionalParameter<"unsigned">:$line,
diff --git a/mlir/lib/CAPI/Dialect/LLVM.cpp b/mlir/lib/CAPI/Dialect/LLVM.cpp
index 13341f0c4de881..ed4d028e192f53 100644
--- a/mlir/lib/CAPI/Dialect/LLVM.cpp
+++ b/mlir/lib/CAPI/Dialect/LLVM.cpp
@@ -159,6 +159,11 @@ MlirAttribute mlirLLVMDIBasicTypeAttrGet(MlirContext ctx, unsigned int tag,
       unwrap(ctx), tag, cast<StringAttr>(unwrap(name)), sizeInBits, encoding));
 }
 
+MlirAttribute mlirLLVMDICompositeTypeAttrGetSelfRec(MlirAttribute recId) {
+  return wrap(
+      DICompositeTypeAttr::getRecSelf(cast<DistinctAttr>(unwrap(recId))));
+}
+
 MlirAttribute mlirLLVMDICompositeTypeAttrGet(
     MlirContext ctx, unsigned int tag, MlirAttribute recId, MlirAttribute name,
     MlirAttribute file, uint32_t line, MlirAttribute scope,
@@ -289,16 +294,21 @@ MlirAttribute mlirLLVMDISubroutineTypeAttrGet(MlirContext ctx,
                           [](Attribute a) { return cast<DITypeAttr>(a); })));
 }
 
+MlirAttribute mlirLLVMDISubprogramAttrGetSelfRec(MlirAttribute recId) {
+  return wrap(DISubprogramAttr::getRecSelf(cast<DistinctAttr>(unwrap(recId))));
+}
+
 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 nRetainedNodes,
-    MlirAttribute const *retainedNodes) {
+    MlirContext ctx, MlirAttribute id, MlirAttribute recId,
+    MlirAttribute compileUnit, MlirAttribute scope, MlirAttribute name,
+    MlirAttribute linkageName, MlirAttribute file, unsigned int line,
+    unsigned int scopeLine, uint64_t subprogramFlags, MlirAttribute type,
+    intptr_t nRetainedNodes, MlirAttribute const *retainedNodes) {
   SmallVector<Attribute> nodesStorage;
   nodesStorage.reserve(nRetainedNodes);
   return wrap(DISubprogramAttr::get(
       unwrap(ctx), cast<DistinctAttr>(unwrap(id)),
+      cast<DistinctAttr>(unwrap(recId)),
       cast<DICompileUnitAttr>(unwrap(compileUnit)),
       cast<DIScopeAttr>(unwrap(scope)), cast<StringAttr>(unwrap(name)),
       cast<StringAttr>(unwrap(linkageName)), cast<DIFileAttr>(unwrap(file)),
@@ -353,14 +363,15 @@ 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) {
+    MlirContext ctx, unsigned int tag, MlirAttribute scope,
+    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)),
+      unwrap(ctx), tag, cast<DIScopeAttr>(unwrap(scope)),
+      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 98a9659735e7e6..0ce719b9a750ca 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -215,6 +215,22 @@ DICompositeTypeAttr::getRecSelf(DistinctAttr recId) {
                                   {}, DIFlags(), 0, 0, {}, {}, {}, {}, {});
 }
 
+//===----------------------------------------------------------------------===//
+// DISubprogramAttr
+//===----------------------------------------------------------------------===//
+
+DIRecursiveTypeAttrInterface DISubprogramAttr::withRecId(DistinctAttr recId) {
+  return DISubprogramAttr::get(
+      getContext(), getId(), recId, getCompileUnit(), getScope(), getName(),
+      getLinkageName(), getFile(), getLine(), getScopeLine(),
+      getSubprogramFlags(), getType(), getRetainedNodes());
+}
+
+DIRecursiveTypeAttrInterface DISubprogramAttr::getRecSelf(DistinctAttr recId) {
+  return DISubprogramAttr::get(recId.getContext(), {}, recId, {}, {}, {}, {}, 0,
+                               0, {}, {}, {}, {});
+}
+
 //===----------------------------------------------------------------------===//
 // TargetFeaturesAttr
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 3870aab52f199d..6e4a964f1fc93c 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -3155,9 +3155,9 @@ struct LLVMOpAsmDialectInterface : public OpAsmDialectInterface {
         .Case<AccessGroupAttr, AliasScopeAttr, AliasScopeDomainAttr,
               DIBasicTypeAttr, DICompileUnitAttr, DICompositeTypeAttr,
               DIDerivedTypeAttr, DIFileAttr, DIGlobalVariableAttr,
-              DIGlobalVariableExpressionAttr, DILabelAttr, DILexicalBlockAttr,
-              DILexicalBlockFileAttr, DILocalVariableAttr, DIModuleAttr,
-              DINamespaceAttr, DINullTypeAttr, DIStringTypeAttr,
+              DIGlobalVariableExpressionAttr, DIImportedEntityAttr, DILabelAttr,
+              DILexicalBlockAttr, DILexicalBlockFileAttr, DILocalVariableAttr,
+              DIModuleAttr, DINamespaceAttr, DINullTypeAttr, DIStringTypeAttr,
               DISubprogramAttr, DISubroutineTypeAttr, LoopAnnotationAttr,
               LoopVectorizeAttr, LoopInterleaveAttr, LoopUnrollAttr,
               LoopUnrollAndJamAttr, LoopLICMAttr, LoopDistributeAttr,
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
index c75f44bf3976a9..ad9aa7ac80cafa 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
@@ -76,8 +76,8 @@ static void addScopeToFunction(LLVM::LLVMFuncOp llvmFunc,
     compileUnitAttr = {};
   }
   auto subprogramAttr = LLVM::DISubprogramAttr::get(
-      context, id, compileUnitAttr, fileAttr, funcNameAttr, funcNameAttr,
-      fileAttr,
+      context, id, /*recId=*/{}, compileUnitAttr, fileAttr, funcNameAttr,
+      funcNameAttr, fileAttr,
       /*line=*/line,
       /*scopeline=*/col, subprogramFlags, subroutineTypeAttr,
       /*retainedNodes=*/{});
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
index ce3643f513d34a..509451057dd737 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -217,8 +217,8 @@ DebugImporter::translateImpl(llvm::DIImportedEntity *node) {
   }
 
   return DIImportedEntityAttr::get(
-      context, node->getTag(), translate(node->getEntity()),
-      translate(node->getFile()), node->getLine(),
+      context, node->getTag(), translate(node->getScope()),
+      translate(node->getEntity()), translate(node->getFile()), node->getLine(),
       getStringAttrOrNull(node->getRawName()), elements);
 }
 
@@ -227,6 +227,7 @@ DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
   mlir::DistinctAttr id;
   if (node->isDistinct())
     id = getOrCreateDistinctID(node);
+
   // Return nullptr if the scope or type is invalid.
   DIScopeAttr scope = translate(node->getScope());
   if (node->getScope() && !scope)
@@ -238,16 +239,19 @@ DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
   if (node->getType() && !type)
     return nullptr;
 
+  // Convert the retained nodes but drop all of them if one of them is invalid.
   SmallVector<DINodeAttr> retainedNodes;
   for (llvm::DINode *retainedNode : node->getRetainedNodes())
     retainedNodes.push_back(translate(retainedNode));
+  if (llvm::is_contained(retainedNodes, nullptr))
+    retainedNodes.clear();
 
-  return DISubprogramAttr::get(context, id, translate(node->getUnit()), scope,
-                               getStringAttrOrNull(node->getRawName()),
-                               getStringAttrOrNull(node->getRawLinkageName()),
-                               translate(node->getFile()), node->getLine(),
-                               node->getScopeLine(), *subprogramFlags, type,
-                               retainedNodes);
+  return DISubprogramAttr::get(
+      context, id, /*recId=*/{}, translate(node->getUnit()), scope,
+      getStringAttrOrNull(node->getRawName()),
+      getStringAttrOrNull(node->getRawLinkageName()),
+      translate(node->getFile()), node->getLine(), node->getScopeLine(),
+      *subprogramFlags, type, retainedNodes);
 }
 
 DISubrangeAttr DebugImporter::translateImpl(llvm::DISubrange *node) {
@@ -374,6 +378,9 @@ getRecSelfConstructor(llvm::DINode *node) {
       .Case([&](llvm::DICompositeType *) {
         return CtorType(DICompositeTypeAttr::getRecSelf);
       })
+      .Case([&](llvm::DISubprogram *) {
+        return CtorType(DISubprogramAttr::getRecSelf);
+      })
       .Default(CtorType());
 }
 
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index 042e015f107fea..55060659dd3560 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -96,6 +96,17 @@ llvm::MDString *DebugTranslation::getMDStringOrNull(StringAttr stringAttr) {
   return llvm::MDString::get(llvmCtx, stringAttr);
 }
 
+llvm::MDTuple *
+DebugTranslation::getMDTupleOrNull(ArrayRef<DINodeAttr> elements) {
+  if (elements.empty())
+    return nullptr;
+  SmallVector<llvm::Metadata *> llvmElements = llvm::to_vector(
+      llvm::map_range(elements, [&](DINodeAttr attr) -> llvm::Metadata * {
+        return translate(attr);
+      }));
+  return llvm::MDNode::get(llvmCtx, llvmElements);
+}
+
 llvm::DIBasicType *DebugTranslation::translateImpl(DIBasicTypeAttr attr) {
   return llvm::DIBasicType::get(
       llvmCtx, attr.getTag(), getMDStringOrNull(attr.getName()),
@@ -138,6 +149,17 @@ DebugTranslation::translateTemporaryImpl(DICompositeTypeAttr attr) {
       /*VTableHolder=*/nullptr);
 }
 
+llvm::TempDISubprogram
+DebugTranslation::translateTemporaryImpl(DISubprogramAttr attr) {
+  return llvm::DISubprogram::getTemporary(
+      llvmCtx, /*Scope=*/nullptr, /*Name=*/{}, /*LinkageName=*/{},
+      /*File=*/nullptr, attr.getLine(), /*Type=*/nullptr,
+      /*ScopeLine=*/0, /*ContainingType=*/nullptr, /*VirtualIndex=*/0,
+      /*ThisAdjustment=*/0, llvm::DINode::FlagZero,
+      static_cast<llvm::DISubprogram::DISPFlags>(attr.getSubprogramFlags()),
+      /*Unit=*/nullptr);
+}
+
 llvm::DICompositeType *
 DebugTranslation::translateImpl(DICompositeTypeAttr attr) {
   // TODO: Use distinct attributes to model this, once they have landed.
@@ -151,10 +173,6 @@ DebugTranslation::translateImpl(DICompositeTypeAttr attr) {
     isDistinct = true;
   }
 
-  SmallVector<llvm::Metadata *> elements;
-  for (DINodeAttr member : attr.getElements())
-    elements.push_back(translate(member));
-
   return getDistinctOrUnique<llvm::DICompositeType>(
       isDistinct, llvmCtx, attr.getTag(), getMDStringOrNull(attr.getName()),
       translate(attr.getFile()), attr.getLine(), translate(attr.getScope()),
@@ -162,7 +180,7 @@ DebugTranslation::translateImpl(DICompositeTypeAttr attr) {
       attr.getAlignInBits(),
       /*OffsetInBits=*/0,
       /*Flags=*/static_cast<llvm::DINode::DIFlags>(attr.getFlags()),
-      llvm::MDNode::get(llvmCtx, elements),
+      getMDTupleOrNull(attr.getElements()),
       /*RuntimeLang=*/0, /*VTableHolder=*/nullptr,
       /*TemplateParams=*/nullptr, /*Identifier=*/nullptr,
       /*Discriminator=*/nullptr,
@@ -242,22 +260,21 @@ DebugTranslation::translateImpl(DIGlobalVariableAttr attr) {
       attr.getIsDefined(), nullptr, nullptr, attr.getAlignInBits(), nullptr);
 }
 
-llvm::DIType *
+llvm::DINode *
 DebugTranslation::translateRecursive(DIRecursiveTypeAttrInterface attr) {
   DistinctAttr recursiveId = attr.getRecId();
-  if (auto *iter = recursiveTypeMap.find(recursiveId);
-      iter != recursiveTypeMap.end()) {
+  if (auto *iter = recursiveNodeMap.find(recursiveId);
+      iter != recursiveNodeMap.end()) {
     return iter->second;
-  } else {
-    assert(!attr.isRecSelf() && "unbound DI recursive self type");
   }
+  assert(!attr.isRecSelf() && "unbound DI recursive self reference");
 
-  auto setRecursivePlaceholder = [&](llvm::DIType *placeholder) {
-    recursiveTypeMap.try_emplace(recursiveId, placeholder);
+  auto setRecursivePlaceholder = [&](llvm::DINode *placeholder) {
+    recursiveNodeMap.try_emplace(recursiveId, placeholder);
   };
 
-  llvm::DIType *result =
-      TypeSwitch<DIRecursiveTypeAttrInterface, llvm::DIType *>(attr)
+  llvm::DINode *result =
+      TypeSwitch<DIRecursiveTypeAttrInterface, llvm::DINode *>(attr)
           .Case<DICompositeTypeAttr>([&](auto attr) {
             auto temporary = translateTemporaryImpl(attr);
             setRecursivePlaceholder(temporary.get());
@@ -266,11 +283,20 @@ DebugTranslation::translateRecursive(DIRecursiveTypeAttrInterface attr) {
             auto *concrete = translateImpl(attr);
             temporary->replaceAllUsesWith(concrete);
             return concrete;
+          })
+          .Case<DISubprogramAttr>([&](auto attr) {
+            auto temporary = translateTemporaryImpl(attr);
+            setRecursivePlaceholder(temporary.get());
+            // Must call `translateImpl` directly instead of `translate` to
+            // avoid handling the recursive interface again.
+            auto *concrete = translateImpl(attr);
+            temporary->replaceAllUsesWith(concrete);
+            return concrete;
           });
 
-  assert(recursiveTypeMap.back().first == recursiveId &&
+  assert(recursiveNodeMap.back().first == recursiveId &&
          "internal inconsistency: unexpected recursive translation stack");
-  recursiveTypeMap.pop_back();
+  recursiveNodeMap.pop_back();
 
   return result;
 }
@@ -297,6 +323,7 @@ llvm::DISubprogram *DebugTranslation::translateImpl(DISubprogramAttr attr) {
 
   bool isDefinition = static_cast<bool>(attr.getSubprogramFlags() &
                                         LLVM::DISubprogramFlags::Definition);
+
   llvm::DISubprogram *node = getDistinctOrUnique<llvm::DISubprogram>(
       isDefinition, llvmCtx, scope, getMDStringOrNull(attr.getName()),
       getMDStringOrNull(attr.getLinkageName()), file, attr.getLine(), type,
@@ -304,21 +331,8 @@ llvm::DISubprogram *DebugTranslation::translateImpl(DISubprogramAttr attr) {
       /*ContainingType=*/nullptr, /*VirtualIndex=*/0,
       /*ThisAdjustment=*/0, llvm::DINode::FlagZero,
       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 (DINodeAttr nodeAttr : attr.getRetainedNodes()) {
-    if (auto 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));
-
+      compileUnit, /*TemplateParams=*/nullptr, /*Declaration=*/nullptr,
+      getMDTupleOrNull(attr.getRetainedNodes()));
   if (attr.getId())
     distinctAttrToNode.try_emplace(attr.getId(), node);
   return node;
@@ -339,16 +353,12 @@ llvm::DINamespace *DebugTranslation::translateImpl(DINamespaceAttr attr) {
                                 attr.getExportSymbols());
 }
 
-llvm::DIImportedEntity *DebugTranslation::translate(DIImportedEntityAttr attr,
-                                                    llvm::DIScope *scope) {
-  SmallVector<llvm::Metadata *> elements;
-  for (DINodeAttr member : attr.getElements())
-    elements.push_back(translate(member));
-
+llvm::DIImportedEntity *
+DebugTranslation::translateImpl(DIImportedEntityAttr attr) {
   return llvm::DIImportedEntity::get(
-      llvmCtx, attr.getTag(), scope, translate(attr.getEntity()),
-      translate(attr.getFile()), attr.getLine(),
-      getMDStringOrNull(attr.getName()), llvm::MDNode::get(llvmCtx, elements));
+      llvmCtx, attr.getTag(), translate(attr.getScope()),
+      translate(attr.getEntity()), translate(attr.getFile()), attr.getLine(),
+      getMDStringOrNull(attr.getName()), getMDTupleOrNull(attr.getElements()));
 }
 
 llvm::DISubrange *DebugTranslation::translateImpl(DISubrangeAttr attr) {
@@ -413,10 +423,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 37b985acf8541e..422aa34e28f3c9 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.h
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.h
@@ -75,6 +75,7 @@ class DebugTranslation {
   llvm::DIDerivedType *translateImpl(DIDerivedTypeAttr attr);
   llvm::DIStringType *translateImpl(DIStringTypeAttr attr);
   llvm::DIFile *translateImpl(DIFileAttr attr);
+  llvm::DIImportedEntity *translateImpl(DIImportedEntityAttr attr);
   llvm::DILabel *translateImpl(DILabelAttr attr);
   llvm::DILexicalBlock *translateImpl(DILexicalBlockAttr attr);
   llvm::DILexicalBlockFile *translateImpl(DILexicalBlockFileAttr attr);
@@ -90,27 +91,26 @@ class DebugTranslation {
   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
   /// method to hook into `translateRecursive`.
   /// - `<temp llvm type> translateTemporaryImpl(<mlir type>)`:
   ///   Create a temporary translation of the DI attr without recursively
   ///   translating any nested DI attrs.
-  llvm::DIType *translateRecursive(DIRecursiveTypeAttrInterface attr);
+  llvm::DINode *translateRecursive(DIRecursiveTypeAttrInterface attr);
 
   /// Translate the given attribute to a temporary llvm debug metadata of the
   /// corresponding type.
   llvm::TempDICompositeType translateTemporaryImpl(DICompositeTypeAttr attr);
+  llvm::TempDISubprogram translateTemporaryImpl(DISubprogramAttr attr);
 
   /// Constructs a string metadata node from the string attribute. Returns
   /// nullptr if `stringAttr` is null or contains and empty string.
   llvm::MDString *getMDStringOrNull(StringAttr stringAttr);
 
+  /// Constructs a tuple metadata node from the `elements`. Returns nullptr if
+  /// `elements` is empty.
+  llvm::MDTuple *getMDTupleOrNull(ArrayRef<DINodeAttr> elements);
+
   /// Constructs a DIExpression metadata node from the DIExpressionAttr. Returns
   /// nullptr if `DIExpressionAttr` is null.
   llvm::DIExpression *getExpressionAttrOrNull(DIExpressionAttr attr);
@@ -125,8 +125,8 @@ class DebugTranslation {
   /// metadata.
   DenseMap<Attribute, llvm::DINode *> attrToNode;
 
-  /// A mapping between recursive ID and the translated DIType.
-  llvm::MapVector<DistinctAttr, llvm::DIType *> recursiveTypeMap;
+  /// A mapping between recursive ID and the translated DINode.
+  llvm::MapVector<DistinctAttr, llvm::DINode *> recursiveNodeMap;
 
   /// A mapping between a distinct ID and the translated LLVM metadata node.
   /// This helps identify attrs that should translate into the same LLVM debug
diff --git a/mlir/test/CAPI/llvm.c b/mlir/test/CAPI/llvm.c
index da28a96f89691d..a4fa2d7a5c9e92 100644
--- a/mlir/test/CAPI/llvm.c
+++ b/mlir/test/CAPI/llvm.c
@@ -248,7 +248,11 @@ static void testDebugInfoAttributes(MlirContext ctx) {
       mlirStringAttrGet(ctx, mlirStringRefCreateFromCString("foo"));
   MlirAttribute bar =
       mlirStringAttrGet(ctx, mlirStringRefCreateFromCString("bar"));
-  MlirAttribute id = mlirDisctinctAttrCreate(foo);
+
+  MlirAttribute none = mlirUnitAttrGet(ctx);
+  MlirAttribute id = mlirDisctinctAttrCreate(none);
+  MlirAttribute recId0 = mlirDisctinctAttrCreate(none);
+  MlirAttribute recId1 = mlirDisctinctAttrCreate(none);
 
   // CHECK: #llvm.di_null_type
   mlirAttributeDump(mlirLLVMDINullTypeAttrGet(ctx));
@@ -312,14 +316,16 @@ static void testDebugInfoAttributes(MlirContext ctx) {
   // CHECK: #llvm.di_subroutine_type<{{.*}}>
   mlirAttributeDump(subroutine_type);
 
+  MlirAttribute di_subprogram_self_rec =
+      mlirLLVMDISubprogramAttrGetSelfRec(recId0);
   MlirAttribute di_imported_entity = mlirLLVMDIImportedEntityAttrGet(
-      ctx, 0, di_module, file, 1, foo, 1, &local_var);
+      ctx, 0, di_subprogram_self_rec, 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,
+      ctx, id, recId0, compile_unit, compile_unit, foo, bar, file, 1, 2, 0,
       subroutine_type, 1, &di_imported_entity);
   // CHECK: #llvm.di_subprogram<{{.*}}>
   mlirAttributeDump(di_subprogram);
@@ -350,10 +356,13 @@ static void testDebugInfoAttributes(MlirContext ctx) {
   // CHECK: #llvm.di_string_type<{{.*}}>
   mlirAttributeDump(string_type);
 
+  // CHECK: #llvm.di_composite_type<tag = DW_TAG_null, recId = {{.*}}>
+  mlirAttributeDump(mlirLLVMDICompositeTypeAttrGetSelfRec(recId1));
+
   // CHECK: #llvm.di_composite_type<{{.*}}>
   mlirAttributeDump(mlirLLVMDICompositeTypeAttrGet(
-      ctx, 0, id, foo, file, 1, compile_unit, di_type, 0, 64, 8, 1, &di_type,
-      expression, expression, expression, expression));
+      ctx, 0, recId1, foo, file, 1, compile_unit, di_type, 0, 64, 8, 1,
+      &di_type, expression, expression, expression, expression));
 }
 
 int main(void) {
diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll
index bb03da37c0d097..59b87d08730f6b 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -307,17 +307,13 @@ define void @class_method() {
   ret void, !dbg !9
 }
 
-; Verify the cyclic composite type is identified, even though conversion begins from the subprogram type.
-; CHECK-DAG: #[[COMP_SELF:.+]] = #llvm.di_composite_type<tag = DW_TAG_null, recId = [[REC_ID:.+]]>
-; CHECK-DAG: #[[COMP_PTR:.+]] = #llvm.di_derived_type<tag = DW_TAG_pointer_type, baseType = #[[COMP_SELF]], sizeInBits = 64>
+; Verify the cyclic subprogram is handled correctly.
+; CHECK-DAG: #[[SP_SELF:.+]] = #llvm.di_subprogram<recId = [[REC_ID:.+]]>
+; CHECK-DAG: #[[COMP:.+]] = #llvm.di_composite_type<tag = DW_TAG_class_type, name = "class_name", file = #{{.*}}, line = 42, flags = "TypePassByReference|NonTrivial", elements = #[[SP_SELF]]>
+; CHECK-DAG: #[[COMP_PTR:.+]] = #llvm.di_derived_type<tag = DW_TAG_pointer_type, baseType = #[[COMP]], sizeInBits = 64>
 ; CHECK-DAG: #[[SP_TYPE:.+]] = #llvm.di_subroutine_type<types = #{{.*}}, #[[COMP_PTR]]>
-; CHECK-DAG: #[[SP_INNER:.+]] = #llvm.di_subprogram<id = [[SP_ID:.+]], compileUnit = #{{.*}}, scope = #[[COMP_SELF]], name = "class_method", file = #{{.*}}, subprogramFlags = Definition, type = #[[SP_TYPE]]>
-; CHECK-DAG: #[[COMP:.+]] = #llvm.di_composite_type<tag = DW_TAG_class_type, recId = [[REC_ID]], name = "class_name", file = #{{.*}}, line = 42, flags = "TypePassByReference|NonTrivial", elements = #[[SP_INNER]]>
-
-; CHECK-DAG: #[[COMP_PTR_OUTER:.+]] = #llvm.di_derived_type<tag = DW_TAG_pointer_type, baseType = #[[COMP]], sizeInBits = 64>
-; CHECK-DAG: #[[SP_TYPE_OUTER:.+]] = #llvm.di_subroutine_type<types = #{{.*}}, #[[COMP_PTR_OUTER]]>
-; CHECK-DAG: #[[SP_OUTER:.+]] = #llvm.di_subprogram<id = [[SP_ID]], compileUnit = #{{.*}}, scope = #[[COMP]], name = "class_method", file = #{{.*}}, subprogramFlags = Definition, type = #[[SP_TYPE_OUTER]]>
-; CHECK-DAG: #[[LOC]] = loc(fused<#[[SP_OUTER]]>
+; CHECK-DAG: #[[SP:.+]] = #llvm.di_subprogram<id = [[SP_ID:.+]], recId = [[REC_ID]], compileUnit = #{{.*}}, scope = #[[COMP]], name = "class_method", file = #{{.*}}, subprogramFlags = Definition, type = #[[SP_TYPE]]>
+; CHECK-DAG: #[[LOC]] = loc(fused<#[[SP]]>
 
 !llvm.dbg.cu = !{!1}
 !llvm.module.flags = !{!0}
@@ -816,4 +812,6 @@ 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"{{.*}}retainedNodes = #llvm.di_imported_entity<tag = DW_TAG_imported_module, entity = #[[M]]{{.*}}>>
+; CHECK-DAG:  #[[SP_REC:.+]] = #llvm.di_subprogram<recId = distinct{{.*}}<>>
+; CHECK-DAG: #[[IE:.+]] = #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #[[SP_REC]], entity = #[[M]]{{.*}}>
+; CHECK-DAG: #[[SP:.+]] = #llvm.di_subprogram<{{.*}}name = "imp_fn"{{.*}}retainedNodes = #[[IE]]>
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index 30b2ba5e9bad1f..f1d7f2a50e9747 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -372,23 +372,28 @@ llvm.func @fn_with_gl() {
 llvm.func @imp_fn() {
   llvm.return
 } 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]<>,
-  sourceLanguage = DW_LANG_Fortran95, file = #file, isOptimized = false,
+
+#di_file = #llvm.di_file<"test.f90" in "">
+#di_subroutine_type = #llvm.di_subroutine_type<callingConvention = DW_CC_program>
+#di_compile_unit = #llvm.di_compile_unit<id = distinct[0]<>,
+  sourceLanguage = DW_LANG_Fortran95, file = #di_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,
-  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>>
+#di_module_1 = #llvm.di_module<file = #di_file, scope = #di_compile_unit, name = "mod1">
+#di_module_2 = #llvm.di_module<file = #di_file, scope = #di_compile_unit, name = "mod2">
+#di_subprogram_self_rec = #llvm.di_subprogram<recId = distinct[1]<>>
+#di_imported_entity_1 = #llvm.di_imported_entity<tag = DW_TAG_imported_module,
+  scope = #di_subprogram_self_rec, entity = #di_module_1, file = #di_file, line = 1>
+#di_imported_entity_2 = #llvm.di_imported_entity<tag = DW_TAG_imported_module,
+  scope = #di_subprogram_self_rec, entity = #di_module_2, file = #di_file, line = 1>
+#di_subprogram = #llvm.di_subprogram<id = distinct[2]<>, recId = distinct[1]<>,
+  compileUnit = #di_compile_unit, scope = #di_file, name = "imp_fn",
+  file = #di_file, subprogramFlags = Definition, type = #di_subroutine_type,
+  retainedNodes = #di_imported_entity_1, #di_imported_entity_2>
 #loc1 = loc("test.f90":12:14)
-#loc2 = loc(fused<#SP>[#loc1])
+#loc2 = loc(fused<#di_subprogram>[#loc1])
 
 // CHECK-DAG: ![[SP:[0-9]+]] = {{.*}}!DISubprogram(name: "imp_fn"{{.*}}retainedNodes: ![[NODES:[0-9]+]])
-// CHECK-DAG: ![[NODES]] = !{![[NODE2:[0-9]+]], ![[NODE1:[0-9]+]]}
+// CHECK-DAG: ![[NODES]] = !{![[NODE1:[0-9]+]], ![[NODE2:[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"{{.*}})
@@ -508,7 +513,7 @@ llvm.func @class_method() {
 llvm.mlir.global @global_variable() {dbg_expr = #di_global_variable_expression} : !llvm.struct<()>
 
 // CHECK: distinct !DIGlobalVariable({{.*}}type: ![[COMP:[0-9]+]],
-// CHECK: ![[COMP]] = distinct !DICompositeType({{.*}}scope: ![[SCOPE:[0-9]+]],
+// CHECK: ![[COMP]] = distinct !DICompositeType({{.*}}scope: ![[SCOPE:[0-9]+]]
 // CHECK: ![[SCOPE]] = !DISubprogram({{.*}}type: ![[SUBROUTINE:[0-9]+]],
 // CHECK: ![[SUBROUTINE]] = !DISubroutineType(types: ![[SR_TYPES:[0-9]+]])
 // CHECK: ![[SR_TYPES]] = !{![[COMP]]}
@@ -540,7 +545,7 @@ llvm.mlir.global @global_variable() {dbg_expr = #di_global_variable_expression}
 // CHECK: distinct !DIGlobalVariable({{.*}}type: ![[VAR:[0-9]+]],
 // CHECK: ![[VAR]] = !DISubroutineType(types: ![[COMPS:[0-9]+]])
 // CHECK: ![[COMPS]] = !{![[COMP:[0-9]+]],
-// CHECK: ![[COMP]] = distinct !DICompositeType({{.*}}scope: ![[SCOPE:[0-9]+]],
+// CHECK: ![[COMP]] = distinct !DICompositeType({{.*}}scope: ![[SCOPE:[0-9]+]]
 // CHECK: ![[SCOPE]] = !DISubprogram({{.*}}type: ![[SUBROUTINE:[0-9]+]],
 // CHECK: ![[SUBROUTINE]] = !DISubroutineType(types: ![[SR_TYPES:[0-9]+]])
 // CHECK: ![[SR_TYPES]] = !{![[COMP]]}



More information about the Mlir-commits mailing list