[5.9][move-only] Fix SIL representation and SILOptimizer to preserve value deinits. #66357
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
atrick
requested review from
gottesmm,
jckarter,
eeckstein,
kavon,
tbkka,
nate-chandler and
meg-gupta
|
@swift-ci test |
gottesmm
approved these changes
Jun 6, 2023
eeckstein
requested changes
Jun 6, 2023
tbkka
approved these changes
Jun 6, 2023
rdar://109863801 ([move-only] DCE removes drop_deinit, so user-defined deinitializers call themselves recursively at -O) (cherry picked from commit af49f62)
(cherry picked from commit cc0c44c)
drop_deinit ultimately only affects the semantics of its destroy_value. Avoid generating releases for destroys in which the deinit has been dropped. Instead, individually release the members. (cherry picked from commit a6f65f0)
drop_deinit only exists in ownership SIL. Remove IRGen support. A drop_deinit can only ever be destroyed or destructured. A destructure of a struct-with-deinit requires a drop_deinit operand. (cherry picked from commit 3efd122)
to MoveOnlyDeinitDevirtualization (cherry picked from commit 9855edd)
It is not needed for correctness and hides most of the deinit related optimizer bugs. (cherry picked from commit 84bee24)
Many basic SIL passes were silently deleting the struct deinit: - SROA - Mem2Reg - RLE - DSE - FunctionSignatureOpts - LowerAggregates Fixes rdar://109849028 ([move-only] LowerAggregateInstrs eliminates struct deinitialization) (cherry picked from commit 4a37b00)
(cherry picked from commit 1dcc240)
atrick
force-pushed
the
5.9-fix-dropdeinit
branch
from
30fe52e to
e2a6e00
Compare
|
@swift-ci test |
|
@swift-ci test |
jckarter
approved these changes
Jun 6, 2023
kavon
reviewed
Jun 6, 2023
kavon
approved these changes
Jun 6, 2023
|
@swift-ci clean test macOS |
|
@swift-ci test |
Multiple code motion and ARC related passes were removing struct/enum deinits. Passes fixed include: - SILCombine - EarlyCodeMotion - ReleaseHoisting - *many* passes that rely on ARC analysis (RCIndentity) (cherry picked from commit 6756ed7)
Fix a special case in visitReleaseValueInst for enum-with-deinit. (cherry picked from commit 58f96a9)
MoveOnlyLoadableStruct should only lower to memberwise destroys if it has no deinit. (cherry picked from commit 986cd42)
(cherry picked from commit 14f5917)
The -Onone passes fail to remove struct_extract, so the CHECK lines will fail. (cherry picked from commit d55ca8a)
atrick
force-pushed
the
5.9-fix-dropdeinit
branch
from
ca051cb to
0fa4d98
Compare
|
@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.
--- CCC ---
Explanation:
Many basic SIL passes were silently deleting the struct and enum deinits:
Major Changes:
Scope of Issue: In some cases, deinitialization would run recursively, hanging the system. In other cases, a struct's deinit would silently fail to run when the struct was a member within another type. In the most trivial cases, with no type composition, and earlier MoveOnlyDeinitInsertion pass was hiding the problem.
Risk: Low risk to regular copyable code. And non-copyable was already broken.
PR to main: [move-only] Fix SIL representation and SILOptimizer to preserve value deinits #66314
#66314
Reviewed By: Erik Eckstein and Joe Groff
Resolves:
rdar://109849028 ([move-only] LowerAggregateInstrs eliminates struct deinitialization)
rdar://109863801 ([move-only] DCE removes drop_deinit, so user-defined deinitializers call themselves recursively at -O)
rdar://110079028 ([move-only] Finish SIL support for drop_deinit)