[DebugInfo] Salvage all stores #72964
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
|
@swift-ci please test |
Snowy1803
force-pushed
the
mem2reg-salvage-store
branch
from
f6deefd to
a4b456b
Compare
|
Looks like the number of lost variables has actually regressed with the last changes. Needs more work |
Snowy1803
force-pushed
the
mem2reg-salvage-store
branch
4 times, most recently
from
f379f65 to
86b6db9
Compare
Snowy1803
force-pushed
the
mem2reg-salvage-store
branch
4 times, most recently
from
4ea59e5 to
190a46c
Compare
Snowy1803
commented
Apr 19, 2024
|
@swift-ci please test |
| @@ -1826,6 +1826,39 @@ void swift::endLifetimeAtLeakingBlocks(SILValue value, | |||
| }); | |||
| } | |||
|
|
|||
| // Replace a store with an appropriate debug value. | |||
| static void transferStoreDebugValue(DebugVarCarryingInst DefiningInst, | |||
There was a problem hiding this comment.
Does DefiningInst imply that this will only work for
%1 = alloc_stack ..., var x
store %1
and not
%1 = alloc_stack ...
debug_value, var x
store %1
?
There was a problem hiding this comment.
It is a DebugVarCarryingInst, so it will be called in both cases: in the first case DefiningInst will be the alloc_stack, in the second case the DefiningInst is the debug_value
Snowy1803
force-pushed
the
mem2reg-salvage-store
branch
2 times, most recently
from
7229561 to
c1e7b49
Compare
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
adrian-prantl
approved these changes
Apr 23, 2024
Snowy1803
force-pushed
the
mem2reg-salvage-store
branch
from
c1e7b49 to
23530a0
Compare
Snowy1803
force-pushed
the
mem2reg-salvage-store
branch
from
23530a0 to
584d1db
Compare
|
I dropped the commit about uninhabited types, it was causing an infinite loop, and it isn't that impactful |
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
| /// Returns this location with the auto-generated and prologue bits stripped. | ||
| /// These bits only make sense for instructions, and should be stripped for | ||
| /// variables. | ||
| SILLocation strippedForDebugVariable() const { |
There was a problem hiding this comment.
Suggested change
| SILLocation strippedForDebugVariable() const { | |
| SILLocation cloneWithoutFlags() const { |
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.
Salvage all stores and store_borrow in salvageDebugInfo. Instead of having a special case in Mem2Reg, we can have a more generic implementation which replaces stores with the expected debug info.
Now that Mem2Reg doesn't drop as much info (99.7% less variables dropped by the pass), this also removes the special case in GenericSpecializer (debug values would otherwise be duplicated)
Also, when salvaging stores, we now use the VarInfo's Loc and Scope properties to indicate the declaration of the variable, and use the location of the store for the location where the debug value is set, instead of using a hacky workaround for arguments only.
This PR also adds special support for salvaging copy_addr in the CopyForwarding pass, using the same system for salvaging stores. CopyForwarding is no longer dropping debug info.
This decreases the number of lost variables in the standard library at the SIL level by 1.3%, and doesn't change significantly the amount of variables at the DWARF level.