Skip to content

[llvm-reduce] Remove DIGlobalVariableExpressions from DICompileUnit's globals #94497

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

Merged
merged 3 commits into from
Jun 6, 2024

Conversation

OCHyams
Copy link
Contributor

@OCHyams OCHyams commented Jun 5, 2024

The 'metadata' delta pass will remove !dbg attachments from globals (which are DIGlobalVariableExpression nodes). The DIGlobalVariableExpressions don't get eliminated from the IR however if they are still referenced by the globals field in DICompileUnit.

Teach the 'di-metadata' pass to try removing global variable operands from metadata tuples as well as DINodes.

… globals

The 'metadata' delta pass will remove !dbg attachments from globals (which are
DIGlobalVariableExpression nodes). The DIGlobalVariableExpressions don't get
eliminated from the IR however if they are still referenced by the globals
field in DICompileUnit.

Teach the 'di-metadata' pass to try removing global variable operands from
metadata tuples as well as DINodes.
@OCHyams OCHyams requested review from nikic, ormris, fhahn and SLTozer June 5, 2024 16:20
@@ -15,18 +15,15 @@
; CHECK-INTERESTINGNESS-DAG: [[SUBPROG]] = distinct !DISubprogram(name: "test",



Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace


TN.push_back(Tup->getOperand(I));
if (Metadata *Op = Tup->getOperand(I).get()) {
if (isa<DINode>(Op) || isa<DIGlobalVariableExpression>(Op)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is possible to find nulls in the MD tree. Given the brute-force approach of this pass, I think isa_and_nonnull makes more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've guarded it with if (Metadata *Op = Tup->getOperand(I).get()) above this line

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes. That's fine, then. One nit: The braces on this if statement are not necessary.

@OCHyams
Copy link
Contributor Author

OCHyams commented Jun 5, 2024

Thanks @ormris - I've addressed your comments

Copy link
Collaborator

@ormris ormris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After one nit, LGTM


TN.push_back(Tup->getOperand(I));
if (Metadata *Op = Tup->getOperand(I).get()) {
if (isa<DINode>(Op) || isa<DIGlobalVariableExpression>(Op)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes. That's fine, then. One nit: The braces on this if statement are not necessary.

@OCHyams OCHyams merged commit c2e62c7 into llvm:main Jun 6, 2024
4 of 6 checks passed
@OCHyams OCHyams deleted the reduce-md branch June 6, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants