Skip to content

Commit 06604a0

Browse files
eecksteineeckstein
authored
Merge pull request #75805 from eeckstein/enable-deinit-devirtualization
Optimizer: enable the DeinitDevirtualizer pass
2 parents 8b66cb5 + 1dab294 commit 06604a0

File tree

12 files changed

+98
-49
lines changed

12 files changed

+98
-49
lines changed

‎SwiftCompilerSources/Sources/Optimizer/FunctionPasses/DeinitDevirtualizer.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ import SIL
1717
let deinitDevirtualizer = FunctionPass(name: "deinit-devirtualizer") {
1818
(function: Function, context: FunctionPassContext) in
1919

20+
guard function.hasOwnership else {
21+
return
22+
}
23+
2024
for inst in function.instructions {
2125
switch inst {
2226
case let destroyValue as DestroyValueInst:

‎SwiftCompilerSources/Sources/Optimizer/Utilities/Devirtualization.swift

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,28 +39,25 @@ private func devirtualize(destroy: some DevirtualizableDestroy, _ context: some
3939
return true
4040
}
4141

42-
let result: Bool
4342
if type.nominal.hasValueDeinit && !destroy.shouldDropDeinit {
4443
guard let deinitFunc = context.lookupDeinit(ofNominal: type.nominal) else {
4544
return false
4645
}
4746
destroy.createDeinitCall(to: deinitFunc, context)
48-
result = true
49-
} else {
50-
// If there is no deinit to be called for the original type we have to recursively visit
51-
// the struct fields or enum cases.
52-
if type.isStruct {
53-
result = destroy.devirtualizeStructFields(context)
54-
} else if type.isEnum {
55-
result = destroy.devirtualizeEnumPayloads(context)
56-
} else {
57-
precondition(type.isClass, "unknown non-copyable type")
58-
// A class reference cannot be further de-composed.
59-
return true
60-
}
47+
context.erase(instruction: destroy)
48+
return true
6149
}
62-
context.erase(instruction: destroy)
63-
return result
50+
// If there is no deinit to be called for the original type we have to recursively visit
51+
// the struct fields or enum cases.
52+
if type.isStruct {
53+
return destroy.devirtualizeStructFields(context)
54+
}
55+
if type.isEnum {
56+
return destroy.devirtualizeEnumPayloads(context)
57+
}
58+
precondition(type.isClass, "unknown non-copyable type")
59+
// A class reference cannot be further de-composed.
60+
return true
6461
}
6562

6663
// Used to dispatch devirtualization tasks to `destroy_value` and `destroy_addr`.
@@ -79,6 +76,10 @@ private extension DevirtualizableDestroy {
7976
guard let cases = type.getEnumCases(in: parentFunction) else {
8077
return false
8178
}
79+
defer {
80+
context.erase(instruction: self)
81+
}
82+
8283
if cases.allPayloadsAreTrivial(in: parentFunction) {
8384
let builder = Builder(before: self, context)
8485
builder.createEndLifetime(of: operand.value)
@@ -122,11 +123,15 @@ extension DestroyValueInst : DevirtualizableDestroy {
122123
}
123124

124125
fileprivate func devirtualizeStructFields(_ context: some MutatingContext) -> Bool {
125-
let builder = Builder(before: self, context)
126-
127126
guard let fields = type.getNominalFields(in: parentFunction) else {
128127
return false
129128
}
129+
130+
defer {
131+
context.erase(instruction: self)
132+
}
133+
134+
let builder = Builder(before: self, context)
130135
if fields.allFieldsAreTrivial(in: parentFunction) {
131136
builder.createEndLifetime(of: operand.value)
132137
return true
@@ -193,6 +198,9 @@ extension DestroyAddrInst : DevirtualizableDestroy {
193198
guard let fields = type.getNominalFields(in: parentFunction) else {
194199
return false
195200
}
201+
defer {
202+
context.erase(instruction: self)
203+
}
196204
if fields.allFieldsAreTrivial(in: parentFunction) {
197205
builder.createEndLifetime(of: operand.value)
198206
return true

‎include/swift/SIL/SILBridgingImpl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1615,7 +1615,7 @@ BridgedInstruction BridgedBuilder::createAllocStack(BridgedType type,
16151615
regularLoc(), type.unbridged(), std::nullopt,
16161616
swift::HasDynamicLifetime_t(hasDynamicLifetime),
16171617
swift::IsLexical_t(isLexical), swift::IsFromVarDecl_t(isFromVarDecl),
1618-
swift::UsesMoveableValueDebugInfo_t(wasMoved))};
1618+
swift::UsesMoveableValueDebugInfo_t(wasMoved), /*skipVarDeclAssert=*/ true)};
16191619
}
16201620

16211621
BridgedInstruction BridgedBuilder::createAllocVector(BridgedValue capacity, BridgedType type) const {

‎lib/SILOptimizer/FunctionSignatureTransforms/OwnedToGuaranteedTransform.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ bool FunctionSignatureTransform::OwnedToGuaranteedAnalyzeParameters() {
8080
if (!A.canOptimizeLiveArg()) {
8181
continue;
8282
}
83+
if (A.Arg->getType().isMoveOnly()) {
84+
// We must not do this transformation for non-copyable types, because it's
85+
// not safe to insert a compensating release_value at the call site. This
86+
// release_value calls the deinit which might have been already de-
87+
// virtualized in the callee.
88+
continue;
89+
}
8390

8491
// See if we can find a ref count equivalent strong_release or release_value
8592
// at the end of this function if our argument is an @owned parameter.

‎lib/SILOptimizer/PassManager/PassPipeline.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,6 @@ static llvm::cl::opt<bool> SILViewSILGenCFG(
6262
"sil-view-silgen-cfg", llvm::cl::init(false),
6363
llvm::cl::desc("Enable the sil cfg viewer pass before diagnostics"));
6464

65-
static llvm::cl::opt<bool>
66-
EnableDeinitDevirtualizer("enable-deinit-devirtualizer", llvm::cl::init(false),
67-
llvm::cl::desc("Enable the DestroyHoisting pass."));
68-
6965
//===----------------------------------------------------------------------===//
7066
// Diagnostic Pass Pipeline
7167
//===----------------------------------------------------------------------===//
@@ -191,14 +187,6 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
191187
P.addLifetimeDependenceDiagnostics();
192188
}
193189

194-
// Devirtualize deinits early if requested.
195-
//
196-
// FIXME: why is DeinitDevirtualizer in the middle of the mandatory pipeline,
197-
// and what passes/compilation modes depend on it? This pass is never executed
198-
// or tested without '-Xllvm enable-deinit-devirtualizer'.
199-
if (EnableDeinitDevirtualizer)
200-
P.addDeinitDevirtualizer();
201-
202190
// As a temporary measure, we also eliminate move only for non-trivial types
203191
// until we can audit the later part of the pipeline. Eventually, this should
204192
// occur before IRGen.
@@ -518,6 +506,8 @@ void addFunctionPasses(SILPassPipelinePlan &P,
518506
}
519507
P.addARCSequenceOpts();
520508

509+
P.addDeinitDevirtualizer();
510+
521511
// We earlier eliminated ownership if we are not compiling the stdlib. Now
522512
// handle the stdlib functions, re-simplifying, eliminating ARC as we do.
523513
if (P.getOptions().CopyPropagation != CopyPropagationOption::Off) {

‎test/IRGen/moveonly_value_functions.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %target-swift-emit-irgen -O \
2+
// RUN: -Xllvm -sil-disable-pass=deinit-devirtualizer \
23
// RUN: -disable-type-layout \
34
// RUN: %s \
45
// RUN: | \

‎test/IRGen/raw_layout_multifile.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %{python} %utils/chex.py < %s > %t/raw_layout_multifile.swift
3-
// RUN: %target-swift-frontend -enable-experimental-feature BuiltinModule -enable-experimental-feature RawLayout -emit-ir -O -disable-availability-checking %s %S/Inputs/raw_layout_multifile_b.swift | %FileCheck %t/raw_layout_multifile.swift --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize
3+
// RUN: %target-swift-frontend -enable-experimental-feature BuiltinModule -enable-experimental-feature RawLayout -emit-ir -O -Xllvm -sil-disable-pass=deinit-devirtualizer -disable-availability-checking %s %S/Inputs/raw_layout_multifile_b.swift | %FileCheck %t/raw_layout_multifile.swift --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize
44

55
@_rawLayout(like: Int32)
66
public struct Foo<T>: ~Copyable {}

‎test/Interpreter/moveonly_swiftskell.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717
// RUN: %target-run %t/E %t/%target-library-name(Swiftskell) \
1818
// RUN: | %FileCheck %s --implicit-check-not destroy
1919

20+
// RUN: %target-build-swift -O -I%t -L%t -lSwiftskell -parse-as-library %s \
21+
// RUN: -module-name Opt -o %t/Opt %target-rpath(%t)
22+
// RUN: %target-codesign %t/Opt
23+
// RUN: %target-run %t/Opt %t/%target-library-name(Swiftskell) | %FileCheck %s --implicit-check-not destroy
24+
2025
// REQUIRES: executable_test
2126

2227
// Temporarily disable for back-deployment (rdar://128544927)

‎test/SILOptimizer/devirt_deinits.sil

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,16 @@ bb0(%0 : $*T):
283283
return %r : $()
284284
}
285285

286+
// CHECK-LABEL: sil @test_non_ossa :
287+
// CHECK: destroy_addr %0
288+
// CHECK: } // end sil function 'test_non_ossa'
289+
sil @test_non_ossa : $@convention(thin) (@in S1) -> () {
290+
bb0(%0 : $*S1):
291+
destroy_addr %0 : $*S1
292+
%r = tuple()
293+
return %r : $()
294+
}
295+
286296
sil @s1_deinit : $@convention(method) (@owned S1) -> ()
287297
sil @s2_deinit : $@convention(method) (@owned S2) -> ()
288298
sil @s3_deinit : $@convention(method) <T> (@in S3<T>) -> ()

‎test/SILOptimizer/devirt_deinits.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
// RUN: %target-swift-frontend -target %target-cpu-apple-macos14 -primary-file %s -parse-as-library -sil-verify-all -Xllvm -enable-deinit-devirtualizer -module-name=test -emit-sil | %FileCheck %s
1+
// RUN: %target-swift-frontend -target %target-cpu-apple-macos14 -primary-file %s -parse-as-library -O -sil-verify-all -module-name=test -Xllvm -sil-disable-pass=function-signature-opts -emit-sil | %FileCheck %s
22

33
// Also do an end-to-end test and check if the compiled executable works as expected.
4-
// RUN: %target-run-simple-swift(-target %target-cpu-apple-macos14 -Xllvm -enable-deinit-devirtualizer -parse-as-library) | %FileCheck -check-prefix CHECK-OUTPUT %s
4+
// RUN: %target-run-simple-swift(-target %target-cpu-apple-macos14 -parse-as-library -O -Xllvm -sil-disable-pass=function-signature-opts) | %FileCheck -check-prefix CHECK-OUTPUT %s
55

66
// Check if it works in embedded mode.
77
// RUN: %target-run-simple-swift(-target %target-cpu-apple-macos14 -enable-experimental-feature Embedded -parse-as-library -runtime-compatibility-version none -wmo -Xfrontend -disable-objc-interop) | %FileCheck -check-prefix CHECK-OUTPUT %s
@@ -98,8 +98,8 @@ func testTwoFieldDeinits(_ s: consuming StrWithoutDeinit) {
9898
}
9999

100100
// CHECK-LABEL: sil hidden [noinline] @$s4test0A5Enum1yyAA2E1OnF :
101-
// CHECK: switch_enum_addr
102-
// CHECK: bb1:
101+
// CHECK: switch_enum
102+
// CHECK: bb1({{.*}}):
103103
// CHECK: [[D:%.*]] = function_ref @$s4test2S1VfD :
104104
// CHECK: apply [[D]]
105105
// CHECK: } // end sil function '$s4test0A5Enum1yyAA2E1OnF'
@@ -109,13 +109,13 @@ func testEnum1(_ s: consuming E1) {
109109
}
110110

111111
// CHECK-LABEL: sil hidden [noinline] @$s4test0A5Enum2yyAA2E2OnF :
112-
// CHECK: switch_enum_addr
113-
// CHECK: bb1:
112+
// CHECK: switch_enum
113+
// CHECK: bb1({{.*}}):
114114
// CHECK: [[D:%.*]] = function_ref @$s4test2S2VfD :
115115
// CHECK: apply [[D]]
116-
// CHECK: bb2:
117-
// CHECK: switch_enum_addr
118-
// CHECK: bb3:
116+
// CHECK: bb2({{.*}}):
117+
// CHECK: switch_enum
118+
// CHECK: bb3({{.*}}):
119119
// CHECK: [[D:%.*]] = function_ref @$s4test2S1VfD :
120120
// CHECK: apply [[D]]
121121
// CHECK: } // end sil function '$s4test0A5Enum2yyAA2E2OnF'

‎test/SILOptimizer/stdlib/Atomics.swift

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ func testInt(_: Int)
1515
// CHECK: [[ATOMIC:%.*]] = alloc_stack [lexical] [var_decl] $Atomic<Int>
1616
// CHECK: [[ATOMIC_PTR:%.*]] = address_to_pointer [[ATOMIC]]
1717
// CHECK: builtin "atomicload_monotonic_Int[[PTR_SIZE]]"([[ATOMIC_PTR]] : $Builtin.RawPointer)
18-
// CHECK: destroy_addr [[ATOMIC]] : $*Atomic<Int>
19-
// CHECK-NEXT: dealloc_stack [[ATOMIC]] : $*Atomic<Int>
18+
// CHECK: dealloc_stack [[ATOMIC]] : $*Atomic<Int>
2019
// CHECK-LABEL: } // end sil function 'localLoad'
2120
@_silgen_name("localLoad")
2221
func localLoad() -> Int {
@@ -28,8 +27,7 @@ func localLoad() -> Int {
2827
// CHECK: [[ATOMIC:%.*]] = alloc_stack [lexical] [var_decl] $Atomic<Int>
2928
// CHECK: [[ATOMIC_PTR:%.*]] = address_to_pointer [[ATOMIC]]
3029
// CHECK: builtin "atomicstore_release_Int[[PTR_SIZE]]"([[ATOMIC_PTR]] : $Builtin.RawPointer
31-
// CHECK: destroy_addr [[ATOMIC]] : $*Atomic<Int>
32-
// CHECK-NEXT: dealloc_stack [[ATOMIC]] : $*Atomic<Int>
30+
// CHECK: dealloc_stack [[ATOMIC]] : $*Atomic<Int>
3331
// CHECK-LABEL: } // end sil function 'localStore'
3432
@_silgen_name("localStore")
3533
func localStore() {
@@ -41,8 +39,7 @@ func localStore() {
4139
// CHECK: [[ATOMIC:%.*]] = alloc_stack [lexical] [var_decl] $Atomic<Int>
4240
// CHECK: [[ATOMIC_PTR:%.*]] = address_to_pointer [[ATOMIC]]
4341
// CHECK: builtin "atomicrmw_xchg_acquire_Int[[PTR_SIZE]]"([[ATOMIC_PTR]] : $Builtin.RawPointer
44-
// CHECK: destroy_addr [[ATOMIC]] : $*Atomic<Int>
45-
// CHECK-NEXT: dealloc_stack [[ATOMIC]] : $*Atomic<Int>
42+
// CHECK: dealloc_stack [[ATOMIC]] : $*Atomic<Int>
4643
// CHECK-LABEL: } // end sil function 'localExchange'
4744
@_silgen_name("localExchange")
4845
func localExchange() -> Int {
@@ -54,8 +51,7 @@ func localExchange() -> Int {
5451
// CHECK: [[ATOMIC:%.*]] = alloc_stack [lexical] [var_decl] $Atomic<Int>
5552
// CHECK: [[ATOMIC_PTR:%.*]] = address_to_pointer [[ATOMIC]]
5653
// CHECK: builtin "cmpxchg_seqcst_seqcst_Int[[PTR_SIZE]]"([[ATOMIC_PTR]] : $Builtin.RawPointer
57-
// CHECK: destroy_addr [[ATOMIC]] : $*Atomic<Int>
58-
// CHECK-NEXT: dealloc_stack [[ATOMIC]] : $*Atomic<Int>
54+
// CHECK: dealloc_stack [[ATOMIC]] : $*Atomic<Int>
5955
// CHECK-LABEL: } // end sil function 'localCompareExchange'
6056
@_silgen_name("localCompareExchange")
6157
func localCompareExchange() -> (exchanged: Bool, original: Int) {

‎test/embedded/non-copyable-deinit.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %target-run-simple-swift(-enable-experimental-feature Embedded -O -parse-as-library -runtime-compatibility-version none -wmo -Xfrontend -disable-objc-interop) | %FileCheck %s
2+
3+
// REQUIRES: executable_test
4+
// REQUIRES: OS=macosx || OS=linux-gnu
5+
6+
var deinitCalled = false
7+
8+
struct S: ~Copyable {
9+
let s: String
10+
11+
@inline(never)
12+
deinit {
13+
precondition(!deinitCalled)
14+
deinitCalled = true
15+
print(s)
16+
}
17+
}
18+
19+
@main
20+
struct Main {
21+
static func main() {
22+
// CHECK: 1
23+
_ = S(s: "1")
24+
// CHECK-NEXT: done
25+
print("done")
26+
}
27+
}
28+

0 commit comments

Comments
 (0)