Skip to content

Commit 8b7a46e

Browse files
eecksteineeckstein
authored
Merge pull request swiftlang#69750 from eeckstein/improve-simplify-destructure
SimplifyDestructure: support simplifying a destructure_struct with a copy_value in place
2 parents f9a5764 + 6ff3b91 commit 8b7a46e

File tree

4 files changed

+147
-22
lines changed

4 files changed

+147
-22
lines changed

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ private func optimizeOptionalBridging(forArgumentOf block: BasicBlock,
8484
}
8585

8686
// Check for the first ObjC -> swift bridging operation.
87-
let swiftValue = lookThroughOwnershipInsts(swiftValueSwitch.enumOp)
87+
let swiftValue = swiftValueSwitch.enumOp.lookThoughOwnershipInstructions
8888
guard let originalObjCValueSwitch = isOptionalBridging(of: swiftValue, isBridging: isBridgeToSwiftCall) else {
8989
return true
9090
}
@@ -126,7 +126,7 @@ private func optimizeNonOptionalBridging(_ apply: ApplyInst,
126126
return true
127127
}
128128

129-
let swiftValue = lookThroughOwnershipInsts(bridgeToObjcCall.arguments[0])
129+
let swiftValue = bridgeToObjcCall.arguments[0].lookThoughOwnershipInstructions
130130

131131
// Handle the first case: the ObjC -> swift bridging operation is optional and the swift -> ObjC
132132
// bridging is within a test for Optional.some, e.g.
@@ -245,18 +245,6 @@ private func removeBridgingCodeInPredecessors(of block: BasicBlock, _ context: F
245245
}
246246
}
247247

248-
private func lookThroughOwnershipInsts(_ value: Value) -> Value {
249-
// Looks like it's sufficient to support begin_borrow and copy_value for now.
250-
// TODO: add move_value if needed.
251-
if let bbi = value as? BeginBorrowInst {
252-
return bbi.borrowedValue
253-
}
254-
if let cvi = value as? CopyValueInst {
255-
return cvi.fromValue
256-
}
257-
return value
258-
}
259-
260248
/// Checks for an optional bridging `switch_enum` diamond.
261249
///
262250
/// ```

‎SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyDestructure.swift

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,55 @@ extension DestructureTupleInst : OnoneSimplifyable {
3131
extension DestructureStructInst : OnoneSimplifyable {
3232
func simplify(_ context: SimplifyContext) {
3333

34-
// Eliminate the redundant instruction pair
35-
// ```
36-
// %s = struct (%0, %1, %2)
37-
// (%3, %4, %5) = destructure_struct %s
38-
// ```
39-
// and replace the results %3, %4, %5 with %0, %1, %2, respectively
40-
//
41-
if let str = self.struct as? StructInst {
34+
switch self.struct {
35+
case let str as StructInst:
36+
// Eliminate the redundant instruction pair
37+
// ```
38+
// %s = struct (%0, %1, %2)
39+
// (%3, %4, %5) = destructure_struct %s
40+
// ```
41+
// and replace the results %3, %4, %5 with %0, %1, %2, respectively
42+
//
4243
tryReplaceConstructDestructPair(construct: str, destruct: self, context)
44+
45+
case let copy as CopyValueInst:
46+
// Similar to the pattern above, but with a copy_value:
47+
// Replace
48+
// ```
49+
// %s = struct (%0, %1, %2)
50+
// %c = copy_value %s // can also be a chain of multiple copies
51+
// (%3, %4, %5) = destructure_struct %c
52+
// ```
53+
// with
54+
// ```
55+
// %c0 = copy_value %0
56+
// %c1 = copy_value %1
57+
// %c2 = copy_value %2
58+
// %s = struct (%0, %1, %2) // keep the original struct
59+
// ```
60+
// and replace the results %3, %4, %5 with %c0, %c1, %c2, respectively.
61+
//
62+
// This transformation has the advantage that we can do it even if the `struct` instruction
63+
// has other uses than the `copy_value`.
64+
//
65+
if copy.uses.singleUse?.instruction == self,
66+
let structInst = copy.fromValue.lookThroughCopy as? StructInst,
67+
structInst.parentBlock == self.parentBlock
68+
{
69+
for (result, operand) in zip(self.results, structInst.operands) {
70+
if operand.value.type.isTrivial(in: parentFunction) {
71+
result.uses.replaceAll(with: operand.value, context)
72+
} else {
73+
let builder = Builder(before: structInst, context)
74+
let copiedOperand = builder.createCopyValue(operand: operand.value)
75+
result.uses.replaceAll(with: copiedOperand, context)
76+
}
77+
}
78+
context.erase(instruction: self)
79+
context.erase(instruction: copy)
80+
}
81+
default:
82+
break
4383
}
4484
}
4585
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,33 @@ extension Value {
1818
uses.lazy.filter { !($0.instruction is DebugValueInst) }
1919
}
2020

21+
var lookThroughBorrow: Value {
22+
if let beginBorrow = self as? BeginBorrowInst {
23+
return beginBorrow.borrowedValue.lookThroughBorrow
24+
}
25+
return self
26+
}
27+
28+
var lookThroughCopy: Value {
29+
if let copy = self as? CopyValueInst {
30+
return copy.fromValue.lookThroughCopy
31+
}
32+
return self
33+
}
34+
35+
var lookThoughOwnershipInstructions: Value {
36+
switch self {
37+
case let beginBorrow as BeginBorrowInst:
38+
return beginBorrow.borrowedValue.lookThoughOwnershipInstructions
39+
case let copy as CopyValueInst:
40+
return copy.fromValue.lookThoughOwnershipInstructions
41+
case let move as MoveValueInst:
42+
return move.fromValue.lookThoughOwnershipInstructions
43+
default:
44+
return self
45+
}
46+
}
47+
2148
/// Walks over all fields of an aggregate and checks if a reference count
2249
/// operation for this value is required. This differs from a simple `Type.isTrivial`
2350
/// check, because it treats a value_to_bridge_object instruction as "trivial".

‎test/SILOptimizer/simplify_destructure_struct.sil

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,74 @@ bb0(%0 : @owned $String, %1 : $Int):
7070
return %4 : $String
7171
}
7272

73+
// CHECK-LABEL: sil [ossa] @forward_with_copy :
74+
// CHECK: %2 = copy_value %0
75+
// CHECK: %3 = struct $S (%0 : $String, %1 : $Int)
76+
// CHECK: fix_lifetime %1
77+
// CHECK: destroy_value %3
78+
// CHECK: return %2
79+
// CHECK: } // end sil function 'forward_with_copy'
80+
sil [ossa] @forward_with_copy : $@convention(thin) (@owned String, Int) -> @owned String {
81+
bb0(%0 : @owned $String, %1 : $Int):
82+
%2 = struct $S (%0 : $String, %1 : $Int)
83+
%3 = copy_value %2 : $S
84+
(%4, %5) = destructure_struct %3 : $S
85+
fix_lifetime %5 : $Int
86+
destroy_value %2 : $S
87+
return %4 : $String
88+
}
89+
90+
// CHECK-LABEL: sil [ossa] @forward_with_two_copies :
91+
// CHECK: %2 = copy_value %0
92+
// CHECK: %3 = struct $S (%0 : $String, %1 : $Int)
93+
// CHECK: %4 = copy_value %3
94+
// CHECK: fix_lifetime %1
95+
// CHECK: destroy_value %3
96+
// CHECK: destroy_value %4
97+
// CHECK: return %2
98+
// CHECK: } // end sil function 'forward_with_two_copies'
99+
sil [ossa] @forward_with_two_copies : $@convention(thin) (@owned String, Int) -> @owned String {
100+
bb0(%0 : @owned $String, %1 : $Int):
101+
%2 = struct $S (%0 : $String, %1 : $Int)
102+
%3 = copy_value %2 : $S
103+
%4 = copy_value %3 : $S
104+
(%5, %6) = destructure_struct %4 : $S
105+
fix_lifetime %6 : $Int
106+
destroy_value %2 : $S
107+
destroy_value %3 : $S
108+
return %5 : $String
109+
}
110+
111+
// CHECK-LABEL: sil [ossa] @copy_has_other_uses :
112+
// CHECK: destructure_struct
113+
// CHECK: } // end sil function 'copy_has_other_uses'
114+
sil [ossa] @copy_has_other_uses : $@convention(thin) (@owned String, Int) -> @owned String {
115+
bb0(%0 : @owned $String, %1 : $Int):
116+
%2 = struct $S (%0 : $String, %1 : $Int)
117+
%3 = copy_value %2 : $S
118+
fix_lifetime %3 : $S
119+
(%5, %6) = destructure_struct %3 : $S
120+
fix_lifetime %6 : $Int
121+
destroy_value %2 : $S
122+
return %5 : $String
123+
}
124+
125+
// CHECK-LABEL: sil [ossa] @different_basic_block :
126+
// CHECK: bb2:
127+
// CHECK: destructure_struct
128+
// CHECK: } // end sil function 'different_basic_block'
129+
sil [ossa] @different_basic_block : $@convention(thin) (@owned String, Int) -> @owned String {
130+
bb0(%0 : @owned $String, %1 : $Int):
131+
%2 = struct $S (%0 : $String, %1 : $Int)
132+
%3 = copy_value %2 : $S
133+
cond_br undef, bb1, bb2
134+
bb1:
135+
destroy_value %3 : $S
136+
unreachable
137+
bb2:
138+
(%4, %5) = destructure_struct %3 : $S
139+
fix_lifetime %5 : $Int
140+
destroy_value %2 : $S
141+
return %4 : $String
142+
}
73143

0 commit comments

Comments
 (0)