Skip to content

Commit 8cea77e

Browse files
eecksteineeckstein
authored
Merge pull request #83913 from eeckstein/fix-deinit-devirtualization-for-arrays
Optimizer: de-virtualize deinits of `builtin "destroyArray"`
2 parents a5a8cf4 + 8e33487 commit 8cea77e

File tree

10 files changed

+172
-22
lines changed

10 files changed

+172
-22
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ let deinitDevirtualizer = FunctionPass(name: "deinit-devirtualizer") {
2727
_ = devirtualizeDeinits(of: destroyValue, context)
2828
case let destroyAddr as DestroyAddrInst:
2929
_ = devirtualizeDeinits(of: destroyAddr, context)
30+
case let builtin as BuiltinInst:
31+
_ = devirtualizeDeinits(of: builtin, context)
3032
default:
3133
break
3234
}

‎SwiftCompilerSources/Sources/Optimizer/ModulePasses/MandatoryPerformanceOptimizations.swift‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,14 @@ private func optimize(function: Function, _ context: FunctionPassContext, _ modu
153153
worklist.addWitnessMethods(of: conformance, moduleContext)
154154

155155
default:
156-
break
156+
if !devirtualizeDeinits(of: bi, simplifyCtxt) {
157+
// If invoked from SourceKit avoid reporting false positives when WMO is turned off for indexing purposes.
158+
if moduleContext.enableWMORequiredDiagnostics {
159+
context.diagnosticEngine.diagnose(.deinit_not_visible, at: bi.location)
160+
}
161+
}
157162
}
158163

159-
160164
// We need to de-virtualize deinits of non-copyable types to be able to specialize the deinitializers.
161165
case let destroyValue as DestroyValueInst:
162166
if !devirtualizeDeinits(of: destroyValue, simplifyCtxt) {

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ func devirtualizeDeinits(of destroy: DestroyAddrInst, _ context: some MutatingCo
2828
return devirtualize(destroy: destroy, context)
2929
}
3030

31+
func devirtualizeDeinits(of builtin: BuiltinInst, _ context: some MutatingContext) -> Bool {
32+
switch builtin.id {
33+
case .DestroyArray:
34+
return devirtualize(builtinDestroyArray: builtin, context)
35+
default:
36+
return true
37+
}
38+
}
39+
3140
private func devirtualize(destroy: some DevirtualizableDestroy, _ context: some MutatingContext) -> Bool {
3241
let type = destroy.type
3342
if !type.isMoveOnly {
@@ -254,6 +263,59 @@ extension DestroyAddrInst : DevirtualizableDestroy {
254263
}
255264
}
256265

266+
private func devirtualize(builtinDestroyArray: BuiltinInst, _ context: some MutatingContext) -> Bool {
267+
let function = builtinDestroyArray.parentFunction
268+
let elementType = builtinDestroyArray.substitutionMap.replacementType.loweredType(in: function)
269+
guard elementType.isMoveOnly,
270+
// This avoids lowering the loop if the element is a non-copyable generic type.
271+
(elementType.isStruct || elementType.isEnum)
272+
else {
273+
return true
274+
}
275+
276+
// Lower the `builtin "destroyArray" to a loop which destroys all elements
277+
278+
let basePointer = builtinDestroyArray.arguments[1]
279+
let arrayCount = builtinDestroyArray.arguments[2]
280+
let indexType = arrayCount.type
281+
let boolType = context.getBuiltinIntegerType(bitWidth: 1)
282+
283+
let preheaderBlock = builtinDestroyArray.parentBlock
284+
let exitBlock = context.splitBlock(after: builtinDestroyArray)
285+
let headerBlock = context.createBlock(after: preheaderBlock)
286+
let bodyBlock = context.createBlock(after: headerBlock)
287+
288+
let preheaderBuilder = Builder(atEndOf: preheaderBlock, location: builtinDestroyArray.location, context)
289+
let zero = preheaderBuilder.createIntegerLiteral(0, type: indexType)
290+
let one = preheaderBuilder.createIntegerLiteral(1, type: indexType)
291+
let falseValue = preheaderBuilder.createIntegerLiteral(0, type: boolType)
292+
let baseAddress = preheaderBuilder.createPointerToAddress(pointer: basePointer,
293+
addressType: elementType.addressType,
294+
isStrict: true, isInvariant: false)
295+
preheaderBuilder.createBranch(to: headerBlock, arguments: [zero])
296+
297+
let inductionVariable = headerBlock.addArgument(type: indexType, ownership: .none, context)
298+
let headerBuilder = Builder(atEndOf: headerBlock, location: builtinDestroyArray.location, context)
299+
let cmp = headerBuilder.createBuiltinBinaryFunction(name: "cmp_slt", operandType: indexType, resultType: boolType,
300+
arguments: [inductionVariable, arrayCount])
301+
headerBuilder.createCondBranch(condition: cmp, trueBlock: bodyBlock, falseBlock: exitBlock)
302+
303+
let bodyBuilder = Builder(atEndOf: bodyBlock, location: builtinDestroyArray.location, context)
304+
let elementAddr = bodyBuilder.createIndexAddr(base: baseAddress, index: inductionVariable,
305+
needStackProtection: false)
306+
let destroy = bodyBuilder.createDestroyAddr(address: elementAddr)
307+
let resultType = context.getTupleType(elements: [indexType, boolType]).loweredType(in: function)
308+
let increment = bodyBuilder.createBuiltinBinaryFunction(name: "sadd_with_overflow", operandType: indexType,
309+
resultType: resultType,
310+
arguments: [inductionVariable, one, falseValue])
311+
let incrResult = bodyBuilder.createTupleExtract(tuple: increment, elementIndex: 0)
312+
bodyBuilder.createBranch(to: headerBlock, arguments: [incrResult])
313+
314+
context.erase(instruction: builtinDestroyArray)
315+
316+
return devirtualize(destroy: destroy, context)
317+
}
318+
257319
private extension EnumCases {
258320
func allPayloadsAreTrivial(in function: Function) -> Bool {
259321
allSatisfy({ $0.payload?.isTrivial(in: function) ?? true })

‎SwiftCompilerSources/Sources/SIL/Builder.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,12 @@ public struct Builder {
567567
}
568568
}
569569

570+
@discardableResult
571+
public func createCondBranch(condition: Value, trueBlock: BasicBlock, falseBlock: BasicBlock) -> CondBranchInst {
572+
let condBr = bridged.createCondBranch(condition.bridged, trueBlock.bridged, falseBlock.bridged)
573+
return notifyNew(condBr.getAs(CondBranchInst.self))
574+
}
575+
570576
@discardableResult
571577
public func createUnreachable() -> UnreachableInst {
572578
let ui = bridged.createUnreachable()

‎SwiftCompilerSources/Sources/SIL/Context.swift‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ extension Context {
4848

4949
public func getBuiltinIntegerType(bitWidth: Int) -> Type { _bridged.getBuiltinIntegerType(bitWidth).type }
5050

51+
public func getTupleType(elements: [Type]) -> AST.`Type` {
52+
let bridgedElements = elements.map { $0.bridged }
53+
return bridgedElements.withBridgedArrayRef {
54+
AST.`Type`(bridged: _bridged.getTupleType($0))
55+
}
56+
}
57+
5158
public var swiftArrayDecl: NominalTypeDecl {
5259
_bridged.getSwiftArrayDecl().getAs(NominalTypeDecl.self)
5360
}

‎include/swift/SIL/SILBridging.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,9 @@ struct BridgedBuilder{
12821282
bool isOnStack = false) const;
12831283
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createBranch(BridgedBasicBlock destBlock,
12841284
BridgedValueArray arguments) const;
1285+
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createCondBranch(BridgedValue condition,
1286+
BridgedBasicBlock trueBlock,
1287+
BridgedBasicBlock falseBlock) const;
12851288
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createUnreachable() const;
12861289
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createObject(BridgedType type, BridgedValueArray arguments,
12871290
SwiftInt numBaseElements) const;
@@ -1425,6 +1428,7 @@ struct BridgedContext {
14251428
SWIFT_IMPORT_UNSAFE OptionalBridgedFunction lookUpNominalDeinitFunction(BridgedDeclObj nominal) const;
14261429
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedSubstitutionMap getContextSubstitutionMap(BridgedType type) const;
14271430
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedType getBuiltinIntegerType(SwiftInt bitWidth) const;
1431+
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedASTType getTupleType(BridgedArrayRef elementTypes) const;
14281432
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedDeclObj getSwiftArrayDecl() const;
14291433
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedDeclObj getSwiftMutableSpanDecl() const;
14301434
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedValue getSILUndef(BridgedType type) const;

‎include/swift/SIL/SILBridgingImpl.h‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2531,6 +2531,13 @@ BridgedInstruction BridgedBuilder::createBranch(BridgedBasicBlock destBlock, Bri
25312531
arguments.getValues(argValues))};
25322532
}
25332533

2534+
BridgedInstruction BridgedBuilder::createCondBranch(BridgedValue condition,
2535+
BridgedBasicBlock trueBlock,
2536+
BridgedBasicBlock falseBlock) const {
2537+
return {unbridged().createCondBranch(regularLoc(), condition.getSILValue(), trueBlock.unbridged(),
2538+
falseBlock.unbridged())};
2539+
}
2540+
25342541
BridgedInstruction BridgedBuilder::createUnreachable() const {
25352542
return {unbridged().createUnreachable(loc.getLoc().getLocation())};
25362543
}
@@ -2847,6 +2854,14 @@ BridgedType BridgedContext::getBuiltinIntegerType(SwiftInt bitWidth) const {
28472854
return swift::SILType::getBuiltinIntegerType(bitWidth, context->getModule()->getASTContext());
28482855
}
28492856

2857+
BridgedASTType BridgedContext::getTupleType(BridgedArrayRef elementTypes) const {
2858+
llvm::SmallVector<swift::TupleTypeElt, 8> elements;
2859+
for (auto bridgedElmtTy : elementTypes.unbridged<BridgedASTType>()) {
2860+
elements.push_back(bridgedElmtTy.unbridged());
2861+
}
2862+
return {swift::TupleType::get(elements, context->getModule()->getASTContext())};
2863+
}
2864+
28502865
BridgedDeclObj BridgedContext::getSwiftArrayDecl() const {
28512866
return {context->getModule()->getASTContext().getArrayDecl()};
28522867
}

‎lib/IRGen/GenType.cpp‎

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
#include "swift/Basic/Platform.h"
2727
#include "swift/Basic/SourceManager.h"
2828
#include "swift/IRGen/Linking.h"
29-
#include "swift/SIL/GenericSpecializationMangler.h"
3029
#include "swift/SIL/SILModule.h"
3130
#include "llvm/IR/DerivedTypes.h"
3231
#include "llvm/ADT/SmallString.h"
@@ -2938,34 +2937,20 @@ static bool tryEmitDeinitCall(IRGenFunction &IGF,
29382937
return true;
29392938
}
29402939

2941-
auto deinitSILFn = deinitTable->getImplementation();
2942-
2943-
// Look for a specialization of deinit that we can call.
2944-
auto substitutions = ty->getContextSubstitutionMap();
2945-
if (!substitutions.empty() &&
2946-
!substitutions.getRecursiveProperties().hasArchetype()) {
2947-
Mangle::GenericSpecializationMangler mangler(
2948-
deinitSILFn->getASTContext(), deinitSILFn,
2949-
deinitSILFn->getSerializedKind());
2950-
2951-
auto specializedName = mangler.mangleReabstracted(
2952-
substitutions, /*needAlternativeMangling=*/false);
2953-
auto specializedFn = IGF.IGM.getSILModule().lookUpFunction(specializedName);
2954-
if (specializedFn)
2955-
deinitSILFn = specializedFn;
2956-
}
2957-
29582940
// The deinit should take a single value parameter of the nominal type, either
29592941
// by @owned or indirect @in convention.
2960-
auto deinitFn = IGF.IGM.getAddrOfSILFunction(deinitSILFn, NotForDefinition);
2961-
auto deinitTy = deinitSILFn->getLoweredFunctionType();
2942+
auto deinitFn = IGF.IGM.getAddrOfSILFunction(deinitTable->getImplementation(),
2943+
NotForDefinition);
2944+
auto deinitTy = deinitTable->getImplementation()->getLoweredFunctionType();
29622945
auto deinitFP = FunctionPointer::forDirect(IGF.IGM, deinitFn,
29632946
nullptr, deinitTy);
29642947
assert(deinitTy->getNumParameters() == 1
29652948
&& deinitTy->getNumResults() == 0
29662949
&& !deinitTy->hasError()
29672950
&& "deinit should have only one parameter");
29682951

2952+
auto substitutions = ty->getContextSubstitutionMap();
2953+
29692954
CalleeInfo info(deinitTy,
29702955
deinitTy->substGenericArgs(IGF.getSILModule(),
29712956
substitutions,

‎test/SILOptimizer/devirt_deinits.sil‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,34 @@ bb0(%0 : $*S1):
304304
return %r : $()
305305
}
306306

307+
// CHECK-LABEL: sil [ossa] @test_destroyArray :
308+
// CHECK: [[ZERO:%.*]] = integer_literal $Builtin.Word, 0
309+
// CHECK: [[ONE:%.*]] = integer_literal $Builtin.Word, 1
310+
// CHECK: [[FALSE:%.*]] = integer_literal $Builtin.Int1, 0
311+
// CHECK: [[PTA:%.*]] = pointer_to_address %0 : $Builtin.RawPointer to [strict] $*S1
312+
// CHECK: br bb1([[ZERO]] : $Builtin.Word)
313+
// CHECK: bb1([[IV:%.*]] : $Builtin.Word):
314+
// CHECK: [[CMP:%.*]] = builtin "cmp_slt_Word"([[IV]] : $Builtin.Word, %1 : $Builtin.Word)
315+
// CHECK: cond_br [[CMP]], bb2, bb3
316+
// CHECK: bb2:
317+
// CHECK: [[ADDR:%.*]] = index_addr [[PTA]] : $*S1, [[IV]]
318+
// CHECK: [[DEINIT:%.*]] = function_ref @s1_deinit :
319+
// CHECK: [[ELEMENT:%.*]] = load [take] [[ADDR]]
320+
// CHECK: apply [[DEINIT]]([[ELEMENT]])
321+
// CHECK: [[ADD:%.*]] = builtin "sadd_with_overflow_Word"([[IV]] : $Builtin.Word, [[ONE]] : $Builtin.Word, [[FALSE]] : $Builtin.Int1)
322+
// CHECK: [[INCR:%.*]] = tuple_extract [[ADD]] : $(Builtin.Word, Builtin.Int1), 0
323+
// CHECK: br bb1([[INCR]] : $Builtin.Word)
324+
// CHECKL bb3:
325+
// CHECK: } // end sil function 'test_destroyArray'
326+
sil [ossa] @test_destroyArray : $@convention(thin) (Builtin.RawPointer, Builtin.Word) -> () {
327+
bb0(%0 : $Builtin.RawPointer, %1 : $Builtin.Word):
328+
%2 = metatype $@thin S1.Type // user: %10
329+
%3 = builtin "destroyArray"<S1>(%2, %0, %1) : $()
330+
%r = tuple ()
331+
return %r
332+
}
333+
334+
307335
sil @s1_deinit : $@convention(method) (@owned S1) -> ()
308336
sil @s2_deinit : $@convention(method) (@owned S2) -> ()
309337
sil @s3_deinit : $@convention(method) <T> (@in S3<T>) -> ()

‎test/SILOptimizer/devirt_deinits.swift‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,16 @@ struct S3: ~Copyable {
5151
}
5252
}
5353

54+
struct S4: ~Copyable {
55+
var x: Int
56+
57+
@inline(never)
58+
deinit {
59+
print(x)
60+
}
61+
}
62+
63+
5464
enum EnumWithDeinit: ~Copyable {
5565
case A(Int)
5666
case B
@@ -136,6 +146,18 @@ func testNestedDeinit(_ s: consuming S3) {
136146
log("### testNestedDeinit")
137147
}
138148

149+
// CHECK-LABEL: sil hidden [noinline] @$s4test0A12DestroyArrayyySryAA2S4VGF :
150+
// CHECK-NOT: "destroyArray"
151+
// CHECK: [[D:%.*]] = function_ref @$s4test2S4VfD :
152+
// CHECK: apply [[D]]
153+
// CHECK-NOT: "destroyArray"
154+
// CHECK: } // end sil function '$s4test0A12DestroyArrayyySryAA2S4VGF'
155+
@inline(never)
156+
func testDestroyArray(_ p: UnsafeMutableBufferPointer<S4>) {
157+
p.deinitialize()
158+
}
159+
160+
139161
@main
140162
struct Main {
141163
static func main() {
@@ -182,6 +204,21 @@ struct Main {
182204
// CHECK-OUTPUT-NEXT: ---
183205
testNestedDeinit(S3())
184206
log("---")
207+
208+
let b = UnsafeMutableBufferPointer<S4>.allocate(capacity: 3)
209+
for i in 0..<3 {
210+
b.initializeElement(at: i, to: S4(x: i))
211+
}
212+
213+
// CHECK-OUTPUT-LABEL: ### testDestroyArray
214+
// CHECK-OUTPUT-NEXT: 0
215+
// CHECK-OUTPUT-NEXT: 1
216+
// CHECK-OUTPUT-NEXT: 2
217+
// CHECK-OUTPUT-NEXT: ---
218+
log("### testDestroyArray")
219+
testDestroyArray(b)
220+
b.deallocate()
221+
log("---")
185222
}
186223
}
187224

0 commit comments

Comments
 (0)