[opt-remark] Infer the source loc retain, release even if they have non-inlined locs. #33820
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
…on-inlined locs. The reason why is that due to ARCCodeMotion, they could have moved enough that the SourceLoc on them is incorrect. That is why you can see in the tests that I had to update, I am moving the retain to the return statement from the body of the function since the retain was now right before the return. I also went in and cleaned up the logic here a little bit. What we do now is that we have a notion of instructions that we /always/ infer SourceLocs for (rr) and ones that if we have a valid non-inlined location we use (e.x.: allocations). This mucked a little bit with my ability to run SIL tests since the SIL tests were relying on this not happening to rr so that we would emit remarks on the rr instructions themselves. I added an option that disables the always infer behavior for this test. That being said at this point to me it seems like the SourceLoc inference stuff is really tied to OptRemarkGenerator and I am going to see if I can move it to there. But that is for a future commit on another day.
|
@swift-ci smoke test |
|
@swift-ci test windows platform |
|
windows test failure isn't this. It is some IRGen test. |
1 similar comment
|
windows test failure isn't this. It is some IRGen 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.
The reason why is that due to ARCCodeMotion, they could have moved enough that
the SourceLoc on them is incorrect. That is why you can see in the tests that I
had to update, I am moving the retain to the return statement from the body of
the function since the retain was now right before the return.
I also went in and cleaned up the logic here a little bit. What we do now is
that we have a notion of instructions that we /always/ infer SourceLocs for (rr)
and ones that if we have a valid non-inlined location we use (e.x.:
allocations).
This mucked a little bit with my ability to run SIL tests since the SIL tests
were relying on this not happening to rr so that we would emit remarks on the rr
instructions themselves. I added an option that disables the always infer
behavior for this test.
That being said at this point to me it seems like the SourceLoc inference stuff
is really tied to OptRemarkGenerator and I am going to see if I can move it to
there. But that is for a future commit on another day.