-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[mlir][debug] Handle DIImportedEntity. #103055
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Add a comment on why this is done in a delayed manner. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
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) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 *); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: This needs a comment, given that it does something non-obvious. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
/// Attributes that support self recursion need to implement an additional | ||
/// method to hook into `translateRecursive`. | ||
/// - `<temp llvm type> translateTemporaryImpl(<mlir type>)`: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a brief comment on the attribute itself why there is no scope field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment on the attribute.