Optimizer: de-virtualize deinits of non-copyable types #69955
Conversation
eeckstein
requested review from
hborla,
slavapestov and
xedin
as code owners
eeckstein
requested review from
atrick and
kavon
and removed request for
xedin,
slavapestov and
hborla
eeckstein
force-pushed
the
deinit-devirtualizer
branch
from
f4a1122 to
a71abd5
Compare
|
@swift-ci test |
|
@swift-ci benchmark |
eeckstein
force-pushed
the
deinit-devirtualizer
branch
from
a71abd5 to
644fc7d
Compare
|
@swift-ci test |
|
@swift-ci benchmark |
There was a problem hiding this comment.
A transformation must not create a
structinstruction with a non-copyable type because this would apply that a (potential) deinit would be called for that struct.
It may not work here, but if you need to create a move-only struct without its deinitializer, you use the pattern:
%dropped drop_deinit %struct
destroy_value %dropped
Implemented by adding a recursive property in TypeLowering
A transformation must not create a `struct` instruction with a non-copyable type because this would apply that a (potential) deinit would be called for that struct.
* add `NominalTypeDecl.isResilient` * make the return type of `Type.getNominalFields` optional and return nil in case the nominal type is resilient. This forces users of this API to think about what to do in case the nominal type is resilient.
It's not needed
…der` * `Type.getEnumCases` * `Builder.createUncheckedTakeEnumDataAddr` * `Builder.createSwitchEnumAddr`
* rename `Context.splitBlock(at:)` -> `Context.splitBlock(before:)` * add `Context.splitBlock(after:)` * add `Context.createBlock(after:)`
In regular swift this is a nice optimization. In embedded swift it's a requirement, because the compiler needs to be able to specialize generic deinits of non-copyable types. The new de-virtualization utilities are called from two places: * from the new DeinitDevirtualizer pass. It replaces the old MoveOnlyDeinitDevirtualization, which is very basic and does not fulfill the needs for embedded swift. * from MandatoryPerformanceOptimizations for embedded swift
eeckstein
force-pushed
the
deinit-devirtualizer
branch
from
644fc7d to
17f246e
Compare
|
@swift-ci smoke test |
| private extension DevirtualizableDestroy { | ||
| var type: Type { operand.value.type } | ||
|
|
||
| var hasDropDeinit: Bool { operand.value.lookThoughOwnershipInstructions is DropDeinitInst } |
There was a problem hiding this comment.
@eeckstein hasDropDeinit should not be called on destroy_addr. There is no requirement that destroy_addr is the only use. And destroy_addr always implies a deinit.
drop_deinit on an address does nothing but prevent the optimizer from reintroducing destroy_addr.
#70052
We should have a separate drop_deinit_addr since it has different semantics.
Remember that, in non-OSSA, there's no connection between allocation and destruction, so things like this are legal:
%1 = alloc_stack
...
destroy_addr %1
destroy_addr %1
We should never see this for non-copyable types, so it's hypothetical at this point.
We could change this and add requirements in SILVerifier. I'm not sure how that would work given that drop_deinit on an address can have other address projection uses.
There was a problem hiding this comment.
hasDropDeinit should not be called on destroy_addr
Thanks! you are right.
drop_deinit on an address does nothing but prevent the optimizer from reintroducing destroy_addr.
See my comments in #70052
We should have a separate drop_deinit_addr since it has different semantics.
yes
| // CHECK: %1 = drop_deinit %0 | ||
| // CHECK: end_lifetime %1 | ||
| // CHECK: } // end sil function 'test_drop_deinit_addr' | ||
| sil [ossa] @test_drop_deinit_addr : $@convention(thin) (@in S1) -> () { |
There was a problem hiding this comment.
test_drop_deinit_addr and test_two_field_drop_deinit_addr are technically incorrect. destroy_addr always calls the deinitializer. Although I don't know how this SIL would occur in practice given that non-copyable types can only have one destroy_addr which will not be emitted if the value has been discarded.
| } | ||
|
|
||
| // Used to dispatch devirtualization tasks to `destroy_value` and `destroy_addr`. | ||
| private protocol DevirtualizableDestroy : UnaryInstruction { |
There was a problem hiding this comment.
You don't need the DevirtualizableDestroy protocol since destructuring only makes sense for destroy_value.
There might be other optimizations where we'll want to destructure a destroy_addr. I just won't be used in this context.
There was a problem hiding this comment.
I'm not sure I understood.
We need to be able to devirtualize a destroy_addr.
In regular swift this is a nice optimization. In embedded swift it's a requirement, because the compiler needs to be able to specialize generic deinits of non-copyable types.
The new de-virtualization utilities are called from two places:
from the new DeinitDevirtualizer pass. It replaces the old MoveOnlyDeinitDevirtualization (which is very basic and does not fulfill the needs for embedded swift). The pass is not yet enabled by default, because it prevents finding deinit-related bugs in the compiler. The pass can be enabled with the
-enable-deinit-devirtualizeroption.from MandatoryPerformanceOptimizations for embedded swift
This PR contains a few other changes: refactoring, small improvements, etc. For details see the commit messages