[move-function] Break blocks right after the non-undef debug info associated with move only values. #41515
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 test |
gottesmm
force-pushed
the
pr-2d2da2179be796c2147236db824f24b137037bdb
branch
from
6e5809e to
693fded
Compare
|
@swift-ci test |
…ociated with move only values. The reason why I am doing this is that right now SelectionDAG seems to sink certain llvm.dbg.addr to the end of block. By breaking the block, we ensure that we initialize the debug value of values early enough. NOTE: We are relying on code at -Onone not eliminating control flow which it does not today. A few additional notes: 1. I also fixed a small issue where I was not setting the proper debug scope for reinit debug_values. This came up when I was writing test cases for this commit. So I just fixed it at the same time. 2. Since I am splitting blocks and I don't want to invalidate dominators/etc at this point in the pipeline, I took advantage of the APIs ability to update dominators/loopinfo at the same time. 3. I also expanded the tests in tree further by adding debug info tests for the reinit copyable/addressonly cases in the multi-block case.
…inst->callMethod() to debug->callMethod(). DebugVarCarryingInst provides this light weight API... lets use it to shrink our code! NFC.
… checker instead of a free function. This lets me simplify the code by eliminating 2 arguments we were already passing in from the checker. NFC.
gottesmm
force-pushed
the
pr-2d2da2179be796c2147236db824f24b137037bdb
branch
from
693fded to
13518c5
Compare
gottesmm
commented
Feb 22, 2022
| @@ -1538,8 +1543,13 @@ bool DataflowState::cleanupAllDestroyAddr( | |||
| if (debugVarInst) { | |||
| if (auto varInfo = debugVarInst.getVarInfo()) { | |||
| SILBuilderWithScope reinitBuilder(*reinit); | |||
| reinitBuilder.createDebugValue(debugVarInst.inst->getLoc(), address, | |||
| *varInfo, false, /*was moved*/ true); | |||
| reinitBuilder.setCurrentDebugScope(debugVarInst->getDebugScope()); | |||
There was a problem hiding this comment.
This is the reinit fix I mention in the commit msg.
gottesmm
commented
Feb 22, 2022
| static bool performSingleBasicBlockAnalysis(DataflowState &dataflowState, | ||
| SILValue address, | ||
| MarkUnresolvedMoveAddrInst *mvi) { | ||
| static bool performSingleBasicBlockAnalysis( |
There was a problem hiding this comment.
In the later commit, I change this to be a method to remove the globals I am passing in here.
|
@swift-ci test |
|
Added a last commit that updates the moveonly_builtin test. |
|
@swift-ci test |
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.
NOTE: This PR contains 3 commits. The initial commit is the meaty commit and I included in the body of this PR msg the commit message from that commit. The other two later commits are just some cleanups to the code that I split from the first commit to make it easier to understand the first commit.
[move-function] Break blocks right after the non-undef debug info associated with move only values.
The reason why I am doing this is that right now SelectionDAG seems to sink
certain llvm.dbg.addr to the end of block. By breaking the block, we ensure that
we initialize the debug value of values early enough. NOTE: We are relying on
code at -Onone not eliminating control flow which it does not today.
A few additional notes:
I also fixed a small issue where I was not setting the proper debug scope for
reinit debug_values. This came up when I was writing test cases for this commit.
So I just fixed it at the same time.
Since I am splitting blocks and I don't want to invalidate dominators/etc at
this point in the pipeline, I took advantage of the APIs ability to update
dominators/loopinfo at the same time.
I also expanded the tests in tree further by adding debug info tests for the
reinit copyable/addressonly cases in the multi-block case.