[move-only] Fix SIL representation and SILOptimizer to preserve value deinits #66314
Conversation
atrick
force-pushed
the
fix-dropdeinit
branch
2 times, most recently
from
3ad3764 to
47eaff5
Compare
| SILValue addr = B.createStructElementAddr( | ||
| cleanupLoc, selfValue, vd, ti.getLoweredType().getAddressType()); | ||
| addr = B.createBeginAccess( | ||
| cleanupLoc, addr, SILAccessKind::Deinit, SILAccessEnforcement::Static, |
There was a problem hiding this comment.
Question. Shouldn't we perhaps do the same thing here for destroy_addr like we do for objects. Then we would be really consistent across the code base.
There was a problem hiding this comment.
drop_deinit on an address doesn't have the same OSSA semantics as it does on a value. The address and value forms definitely should be separate opcodes to avoid this confusion (in some later PR).
It is analogous to destroy_addr vs. destroy_value. destroy_addr has nothing to do with OSSA, and drop_deinit on an address does not change the meaning of any subsequent destroy_addr. drop_deinit on an address does not actually do anything except block optimization on the address, preventing us from promoting it to OSSA!
So drop_deinit on an address doesn't have any SIL requirements and has no structural verification. As such, SILGen cannot emit a destroy_addr for a discarded value. That would be outright wrong. And OSSA lowering is not affected at all by a drop_deinit on an address.
atrick
force-pushed
the
fix-dropdeinit
branch
4 times, most recently
from
eff1206 to
f62e5f6
Compare
|
@swift-ci test |
atrick
changed the title
|
@swift-ci smoke test windows |
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.
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.
to MoveOnlyDeinitDevirtualization
It is not needed for correctness and hides most of the deinit related optimizer bugs.
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)
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)
Fix a special case in visitReleaseValueInst for enum-with-deinit.
MoveOnlyLoadableStruct should only lower to memberwise destroys if it has no deinit.
The -Onone passes fail to remove struct_extract, so the CHECK lines will fail.
|
@swift-ci smoke test |
Fix SIL representation and SILOptimizer to preserve value deinits.
Fixes:
Many basic SIL passes were silently deleting the struct and enum deinits:
Major Changes: