[mlir][debug] Handle DIImportedEntity. #103055
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dinistro
reviewed
Aug 13, 2024
There was a problem hiding this comment.
Is there any restriction on the scope of a di_imported_entity?
In general, I wonder if it would be better to attach them to the DISubprogramAttr instead, but that leads to yet another way one can get a cyclic attribute (cc @zyx-billy).
Comment on lines
630
to
641
| OptionalParameter<"DIScopeAttr">:$scope, | ||
| OptionalParameter<"DINodeAttr">:$entity, | ||
| OptionalParameter<"DIFileAttr">:$file, | ||
| OptionalParameter<"unsigned">:$line, | ||
| OptionalParameter<"StringAttr">:$name, | ||
| OptionalArrayRefParameter<"DINodeAttr">:$elements |
There was a problem hiding this comment.
Does LLVM really make all these fields optional?
|
|
||
| ; 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]] |
There was a problem hiding this comment.
Can you check that this attribute is attached to the expected place in the IR?
I am not aware of any restriction on the scope. I think it can take any Please correct me if I am wrong but I thought Attributes are sort of readonly once created. So adding a field in
Similarly, we can't attach attribute or location to |
|
I agree that it would be desired to replicate LLVM's structure and add the new attribute using a retained nodes field in the subprogram.
There is a mechanism to do this in principle but it is only implement for DICompositeType so far. Essentially DISubprogram would have to implement the LLVM_DIRecursiveTypeAttrInterface ( ). When translating you would then start translating the subprogram retained nodes but you remember that the traversal already saw the subprogram once and give it an id. So the DIImportedEntity scope would only contain an id instead of the full subprogram. Once all nested attributes are translated you then create the actual subprogram which has the id attached.I believe we will need some consulting from @zyx-billy to get this working though (the mechanism is non-trivial). In principle the import of debug info already has the mechanism working but only for DICompositeType. |
|
https://discourse.llvm.org/t/handling-cyclic-dependencies-in-debug-info/67526/17 discusses the topic. I think it may make sense to look at the last posts of Billy and look at the relevant PRs. |
|
Thank you @gysit for the overview. Yes, unfortunately this is our only solution for cyclic DI metadata right now. Coincidentally we were just discussing in #98203 how we may need to add recursion support so that DISubprogram may be used to "prune" cycles, and once that's added this should work automatically, but neither of us got to it yet... Another solution, if an ImportedEntity is very limited in where it can exist, is to forgo the back-referencing "scope" field from the ImportedEntity and just rely on the context when exporting back to llvm to assign it the scope again. Is there such a guarantee on where an ImportedEntity can appear? |
|
Thanks @gysit @zyx-billy and @Dinistro for your comments. @zyx-billy the idea to remove the scope field from the imported entity is interesting one. This seems like something that could be done as scope could be inferred depending on where these entities are stores. I will go and experiment on it. |
|
Hi @zyx-billy, @gysit. @Dinistro |
Dinistro
reviewed
Aug 19, 2024
| @@ -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, {}); | |||
There was a problem hiding this comment.
Nit: Also add the param name, like it's done for line and scopeline here.
| @@ -208,6 +208,21 @@ DINamespaceAttr DebugImporter::translateImpl(llvm::DINamespace *node) { | |||
| node->getExportSymbols()); | |||
| } | |||
|
|
|||
| DIImportedEntityAttr | |||
There was a problem hiding this comment.
This requires a clear TODO comment that we drop this information that also explains why it is dropped. Ultimately, we would want to import even the cyclic structures, but this can wait for now.
There was a problem hiding this comment.
Maybe add a brief comment on the attribute itself why there is no scope field.
There was a problem hiding this comment.
I added a comment on the attribute.
| @@ -306,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()) { | |||
There was a problem hiding this comment.
Nit: Do not use auto here.
There was a problem hiding this comment.
Thanks for catching this. Done.
| @@ -306,6 +306,18 @@ llvm::DISubprogram *DebugTranslation::translateImpl(DISubprogramAttr attr) { | |||
| static_cast<llvm::DISubprogram::DISPFlags>(attr.getSubprogramFlags()), | |||
| compileUnit); | |||
|
|
|||
| SmallVector<llvm::Metadata *> retainedNodes; | |||
There was a problem hiding this comment.
Nit: Add a comment on why this is done in a delayed manner.
| @@ -89,6 +89,7 @@ class DebugTranslation { | |||
| llvm::DISubrange *translateImpl(DISubrangeAttr attr); | |||
| llvm::DISubroutineType *translateImpl(DISubroutineTypeAttr attr); | |||
| llvm::DIType *translateImpl(DITypeAttr attr); | |||
| llvm::DIImportedEntity *translate(DIImportedEntityAttr attr, llvm::DIScope *); | |||
There was a problem hiding this comment.
Nit: This needs a comment, given that it does something non-obvious.
gysit
reviewed
Aug 19, 2024
mlir/include/mlir-c/Dialect/LLVM.h
Outdated
Comment on lines
319
to
320
| uint64_t subprogramFlags, MlirAttribute type, intptr_t nNodes, | ||
| MlirAttribute const *nodes); |
There was a problem hiding this comment.
Suggested change
| uint64_t subprogramFlags, MlirAttribute type, intptr_t nNodes, | |
| MlirAttribute const *nodes); | |
| uint64_t subprogramFlags, MlirAttribute type, intptr_t nRetainedNodes, | |
| MlirAttribute const *retainedNodes); |
nit: The name should probably match the attribute fields?
| SmallVector<llvm::Metadata *> retainedNodes; | ||
|
|
||
| for (auto nodeAttr : attr.getRetainedNodes()) { | ||
| if (DIImportedEntityAttr importedAttr = |
There was a problem hiding this comment.
Suggested change
| if (DIImportedEntityAttr importedAttr = | |
| if (auto importedAttr = |
| @@ -208,6 +208,21 @@ DINamespaceAttr DebugImporter::translateImpl(llvm::DINamespace *node) { | |||
| node->getExportSymbols()); | |||
| } | |||
|
|
|||
| DIImportedEntityAttr | |||
There was a problem hiding this comment.
Maybe add a brief comment on the attribute itself why there is no scope field.
Comment on lines
242
to
244
| SmallVector<DINodeAttr> retainedNodes; | ||
|
|
||
| for (auto node : node->getRetainedNodes()) |
There was a problem hiding this comment.
Suggested change
| SmallVector<DINodeAttr> retainedNodes; | |
| for (auto node : node->getRetainedNodes()) | |
| SmallVector<DINodeAttr> retainedNodes; | |
| for (auto node : node->getRetainedNodes()) |
Comment on lines
213
to
214
| SmallVector<DINodeAttr> elements; | ||
|
|
||
| for (llvm::DINode *element : node->getElements()) { |
There was a problem hiding this comment.
Suggested change
| SmallVector<DINodeAttr> elements; | |
| for (llvm::DINode *element : node->getElements()) { | |
| SmallVector<DINodeAttr> elements; | |
| for (llvm::DINode *element : node->getElements()) { |
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Abid Qadeer (abidh) ChangesThe This PR adds When an entity is imported in a function, the This PR makes sure that the translation from mlir to There was no obvious place where I could place the This PR currently does not handle entities imported in a global scope but that should be easy to handle in a subsequent PR. Full diff: https://github.com/llvm/llvm-project/pull/103055.diff 13 Files Affected:
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 3c067bf946cfc9..8d8844e7eac38b 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -300,7 +300,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.
diff --git a/mlir/include/mlir-c/Dialect/LLVM.h b/mlir/include/mlir-c/Dialect/LLVM.h
index 631b5646183204..72741331d6c7ee 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 nRetainedNodes,
+ MlirAttribute const *retainedNodes);
+
/// 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 5d96f506342588..e57be7f760d380 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);
}]>
];
@@ -619,6 +621,29 @@ def LLVM_DINamespaceAttr : LLVM_Attr<"DINamespace", "di_namespace",
let assemblyFormat = "`<` struct(params) `>`";
}
+//===----------------------------------------------------------------------===//
+// DIImportedEntityAttr
+//===----------------------------------------------------------------------===//
+
+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,
+ OptionalParameter<"DIFileAttr">:$file,
+ OptionalParameter<"unsigned">:$line,
+ OptionalParameter<"StringAttr">:$name,
+ OptionalArrayRefParameter<"DINodeAttr">:$elements
+ );
+
+ let assemblyFormat = "`<` struct(params) `>`";
+}
+
//===----------------------------------------------------------------------===//
// DISubrangeAttr
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/CAPI/Dialect/LLVM.cpp b/mlir/lib/CAPI/Dialect/LLVM.cpp
index 03e2f2be2156af..bc1b8f38c5bdf0 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,17 @@ 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 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(nRetainedNodes, retainedNodes, nodesStorage),
+ [](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..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 1817c1271b43ee..ce3643f513d34a 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -208,6 +208,20 @@ 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->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;
@@ -223,11 +237,17 @@ DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
DISubroutineTypeAttr type = translate(node->getType());
if (node->getType() && !type)
return nullptr;
+
+ SmallVector<DINodeAttr> retainedNodes;
+ for (llvm::DINode *retainedNode : node->getRetainedNodes())
+ retainedNodes.push_back(translate(retainedNode));
+
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) {
@@ -308,6 +328,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..042e015f107fea 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -306,6 +306,19 @@ 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 (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));
+
if (attr.getId())
distinctAttrToNode.try_emplace(attr.getId(), node);
return node;
@@ -326,6 +339,18 @@ 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));
+
+ return llvm::DIImportedEntity::get(
+ llvmCtx, attr.getTag(), scope, 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)
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.h b/mlir/lib/Target/LLVMIR/DebugTranslation.h
index 16a853736226da..37b985acf8541e 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.h
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.h
@@ -90,6 +90,12 @@ 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>)`:
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 03c3855a9a324c..bb03da37c0d097 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -792,3 +792,28 @@ 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"{{.*}}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 1a9a8561de00dc..30b2ba5e9bad1f 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -366,6 +366,36 @@ llvm.func @fn_with_gl() {
// -----
+// Test that imported entries correctly generates 'retainedNodes' in the
+// subprogram.
+
+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,
+ 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>>
+#loc1 = loc("test.f90":12:14)
+#loc2 = loc(fused<#SP>[#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: ![[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]
|
|
Fixing the build in this PR is fine. |
|
Is this related to issue #98883? |
Yes, this is mlir side of the work. Once this is in then I will open a PR for flang side of the fix. |
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.
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.
Mostly formatting nits and adding comments.
Add a default value for retainedNodes field.
The function parameter names were swapped in 2 places.
abidh
force-pushed
the
mlir_imported_module
branch
from
880315b to
aea8311
Compare
|
Cannot check right now, but if @gysit is happy, I'm happy. |
|
@abidh We're are experiencing a crash in the cyclic import cache (cc @zyx-billy) after this change. I'm unsure if this warrants a revert, since this might be a bug uncovered in the caching code? (cc @Dinistro and @gysit). Attaching a reproducer below: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
DIImporedEntitycan be used to represent imported entities like C++'s namespace with using directive or fortran's moudule with use statement.This PR adds
DIImportedEntityAttrand 2-way translation fromDIImportedEntitytoDIImportedEntityAttrand vice versa.When an entity is imported in a function, the
retainedNodesfield of theDISubprogramcontains all the imported nodes. See the C++ code and the LLVM IR below.This PR makes sure that the translation from mlir to
retainedNodesfield happens correctly both ways.There was no obvious place where I could place the
DIImportedEntityAttrin the mlir. I have decided to piggy back on theFusedLocofFuncOpwhich is already used to carryDISubprogramAttr. 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.