Skip to content

Retain more debug info in optimized builds #85244

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.

Sign up for GitHub

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

Jump to bottom
Merged
aidan-hall merged 1 commit into from
Dec 1, 2025
Merged

Retain more debug info in optimized builds #85244

aidan-hall merged 1 commit into from
Dec 1, 2025

Conversation

@aidan-hall
Copy link
Contributor

@aidan-hall aidan-hall commented Oct 31, 2025 •
edited
Loading

Depends on #85237: Ignore b353bb3 in this PR.

@aidan-hall aidan-hall self-assigned this Oct 31, 2025
@aidan-hall aidan-hall changed the title Fixing debug info rebase Retain more debug info in optimized builds Oct 31, 2025
@aidan-hall aidan-hall mentioned this pull request Oct 31, 2025
Closed
@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch 3 times, most recently from d1aece3 to 53b21e3 Compare November 5, 2025 16:08
@aidan-hall aidan-hall marked this pull request as ready for review November 5, 2025 16:37
adrian-prantl
adrian-prantl approved these changes Nov 5, 2025
View reviewed changes
Copy link
Contributor

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

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

This is great!

@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch 3 times, most recently from 3faa1e9 to fb593f4 Compare November 5, 2025 18:08
eeckstein
eeckstein reviewed Nov 6, 2025
View reviewed changes
SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift Outdated
}
}

first.salvageDebugInfo(self)
Copy link
Contributor

@eeckstein eeckstein Nov 6, 2025

Choose a reason for hiding this comment

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

I'm wondering why this is needed.
The following replace calls erase(instruction:) which itself calls salvageDebugInfo

Copy link
Contributor Author

@aidan-hall aidan-hall Nov 10, 2025

Choose a reason for hiding this comment

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

The replace is for the second instruction, while this salvage is for the first instruction. It should only be necessary in optimized builds, so I'm changing this code. The test for this case is DebugInfo/simplify_struct_extract.sil.

Copy link
Contributor

@eeckstein eeckstein Nov 11, 2025

Choose a reason for hiding this comment

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

Ah, okay

@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch 3 times, most recently from 118f114 to af515fd Compare November 10, 2025 11:51
aidan-hall
aidan-hall commented Nov 10, 2025
View reviewed changes
Copy link
Contributor Author

@aidan-hall aidan-hall left a comment

Choose a reason for hiding this comment

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

one point of confusion

@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 10, 2025

@swift-ci smoke test

1 similar comment
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 11, 2025

@swift-ci smoke test

@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch from af515fd to 9240379 Compare November 11, 2025 18:15
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 11, 2025

macOS & Windows passed, but Linux had a crash in my new salvageDebugInfo case. I smell a subtle incompatibility between libstdc++ and libc++ std::optional, so I've adjusted how I'm using it to be consistent with the other cases in the same function.
@swift-ci smoke test

@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 12, 2025

@swift-ci smoke test linux

@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 12, 2025

@swift-ci smoke test macos

@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch 3 times, most recently from 9d9dcc0 to 8c1debd Compare November 17, 2025 10:47
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 17, 2025

@swift-ci smoke test

@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 17, 2025

@swift-ci test windows

@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch from 8c1debd to e80a0f5 Compare November 18, 2025 11:40
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 18, 2025

@swift-ci smoke test linux

@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch from e80a0f5 to 7e8b65e Compare November 19, 2025 11:10
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 19, 2025

@swift-ci smoke test linux

@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch from 7e8b65e to 966d4ef Compare November 19, 2025 17:43
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 19, 2025

@swift-ci smoke test linux

@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 20, 2025

@swift-ci smoke test

@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch from 966d4ef to 7e8b65e Compare November 21, 2025 12:06
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 21, 2025

@swift-ci smoke test linux

@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch from 7e8b65e to eb9071c Compare November 21, 2025 14:38
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 21, 2025

@swift-ci smoke test

@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 24, 2025

@swift-ci smoke test linux

@aidan-hall aidan-hall enabled auto-merge November 25, 2025 10:19
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 25, 2025

@swift-ci test

@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 27, 2025

@swift-ci test macos

@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 28, 2025

@swift-ci test windows

@aidan-hall
[DebugInfo] Salvage more in -O builds
a95d297 
Specifically, improved debug info retention in:
* tryReplaceRedundantInstructionPair,
* splitAggregateLoad,
* TempLValueElimination,
* Mem2Reg,
* ConstantFolding.

The changes to Mem2Reg allow debug info to be retained in the case tested by
self-nostorage.swift in -O builds, so we have just enabled -O in that file
instead of writing a new test for it.

We attempted to add a case to salvageDebugInfo for unchecked_enum_data, but it
caused crashes in Linux CI that we were not able to reproduce.
@aidan-hall aidan-hall force-pushed the fixing-debug-info-rebase branch from eb9071c to a95d297 Compare November 28, 2025 17:42
@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 28, 2025

@swift-ci smoke test

@aidan-hall
Copy link
Contributor Author

aidan-hall commented Nov 28, 2025

The Linux & MacOS validation tests succeeded before that last force-push, which was just a whitespace change to unblock the Windows tests, so we don't need to run those again.

@shahmishal
Copy link
Member

shahmishal commented Dec 1, 2025

@swift-ci test windows

3 similar comments
@shahmishal
Copy link
Member

shahmishal commented Dec 1, 2025

@swift-ci test windows

@shahmishal
Copy link
Member

shahmishal commented Dec 1, 2025

@swift-ci test windows

@shahmishal
Copy link
Member

shahmishal commented Dec 1, 2025

@swift-ci test windows

@aidan-hall aidan-hall merged commit 96dca43 into swiftlang:main Dec 1, 2025
3 checks passed
@aidan-hall aidan-hall mentioned this pull request Dec 10, 2025
Open
aidan-hall added a commit to aidan-hall/swift that referenced this pull request Dec 10, 2025
@aidan-hall
[DebugInfo] Salvage more during SIL optimization passes
e797b25 
This reverts commit b1eb70b.

The original PR (swiftlang#85244) was reverted (swiftlang#85836) due to rdar://165667449

This version fixes the aforementioned issue, and (potentially) improves overall
debug info retention by salvaging info in erase(instructionIncludingDebugUses:),
rather than inserting many explicit salvageDebugInfo calls throughout the code
to make the test cases pass.

Bridging: Make salvageDebugInfo a method of MutatingContext

This feels more consistent than making it a method of Instruction.

DebugInfo: Salvage from trivially dead instructions

This handles the case we were checking in constantFoldBuiltin, so we do not need to salvage debug info there any more.
aidan-hall added a commit to aidan-hall/swift that referenced this pull request Dec 10, 2025
@aidan-hall
[DebugInfo] Salvage more during SIL optimization passes
1cb333a 
This reverts commit b1eb70b.

The original PR (swiftlang#85244) was reverted (swiftlang#85836) due to rdar://165667449

This version fixes the aforementioned issue, and (potentially) improves overall
debug info retention by salvaging info in erase(instructionIncludingDebugUses:),
rather than inserting many explicit salvageDebugInfo calls throughout the code
to make the test cases pass.

Bridging: Make salvageDebugInfo a method of MutatingContext

This feels more consistent than making it a method of Instruction.

DebugInfo: Salvage from trivially dead instructions

This handles the case we were checking in constantFoldBuiltin, so we do not need to salvage debug info there any more.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eeckstein eeckstein eeckstein left review comments

@adrian-prantl adrian-prantl adrian-prantl approved these changes

@jckarter jckarter Awaiting requested review from jckarter jckarter is a code owner

Assignees

@aidan-hall aidan-hall

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@aidan-hall @shahmishal @adrian-prantl @eeckstein