Skip to content

Commit 334b988

Browse files
nate-chandlernate-chandler
committed
[ConsumeObjectChecker] End lifetimes at consumes.
The checker already verifies that no non-destroy consuming users occur after any `move_value`s corresponding to `consume` operators applied to a value. There may, however, be _destroy_ users after it. Previously, the checker did not shorten the lifetime from those destroys up to `move_value`s that appear after those `move_value`s. The result was that the value's lifetime didn't end at the `consume`. Here, the checker is fixed to rewrite the lifetimes so that they both end at `consume`s and also maintain their lexical lifetimes on paths away from the `consume`s. This is done by using `OwnedValueCanonicalization`/`CanonicalizeOSSALifetime`. Specifically, it passes the `move_value`s that correspond to source-level `consume`s as the `lexicalLifetimeEnds` to the canonicalizer. Typically, the canonicalizer retracts the lexical lifetime of the value from its destroys. When these `move_value`s are specified, however, instead it retracts them from the lifetime boundary obtained by maximizing the lifetime within its original lifetime while maintaining the property that the lifetime ends at those `move_value`s. rdar://113142446
1 parent db5e0cf commit 334b988

File tree

3 files changed

+253
-19
lines changed

3 files changed

+253
-19
lines changed

‎lib/SILOptimizer/Mandatory/ConsumeOperatorCopyableValuesChecker.cpp

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "swift/SIL/SILInstruction.h"
2626
#include "swift/SIL/SILUndef.h"
2727
#include "swift/SILOptimizer/Analysis/Analysis.h"
28+
#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
2829
#include "swift/SILOptimizer/Analysis/ClosureScope.h"
2930
#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
3031
#include "swift/SILOptimizer/Analysis/LoopAnalysis.h"
@@ -216,17 +217,18 @@ namespace {
216217
struct ConsumeOperatorCopyableValuesChecker {
217218
SILFunction *fn;
218219
CheckerLivenessInfo livenessInfo;
219-
DominanceInfo *dominanceToUpdate;
220-
SILLoopInfo *loopInfoToUpdate;
221-
222-
ConsumeOperatorCopyableValuesChecker(SILFunction *fn)
223-
: fn(fn), dominanceToUpdate(nullptr), loopInfoToUpdate(nullptr) {}
224-
225-
void setDominanceToUpdate(DominanceInfo *newDFI) {
226-
dominanceToUpdate = newDFI;
227-
}
228-
229-
void setLoopInfoToUpdate(SILLoopInfo *newLFI) { loopInfoToUpdate = newLFI; }
220+
DominanceInfo *dominance;
221+
InstructionDeleter deleter;
222+
CanonicalizeOSSALifetime canonicalizer;
223+
224+
ConsumeOperatorCopyableValuesChecker(SILFunction *fn,
225+
DominanceInfo *dominance,
226+
BasicCalleeAnalysis *calleeAnalysis)
227+
: fn(fn), dominance(dominance),
228+
canonicalizer(DontPruneDebugInsts,
229+
MaximizeLifetime_t(!fn->shouldOptimize()), fn,
230+
/*accessBlockAnalysis=*/nullptr, dominance,
231+
calleeAnalysis, deleter) {}
230232

231233
bool check();
232234

@@ -480,6 +482,7 @@ bool ConsumeOperatorCopyableValuesChecker::check() {
480482
// Then look at all of our found consuming uses. See if any of these are
481483
// _move that are within the boundary.
482484
bool foundMove = false;
485+
SmallVector<SILInstruction *, 2> validMoves;
483486
auto dbgVarInst = DebugVarCarryingInst::getFromValue(lexicalValue);
484487
StringRef varName = DebugVarCarryingInst::getName(dbgVarInst);
485488
for (auto *use : livenessInfo.consumingUse) {
@@ -510,6 +513,7 @@ bool ConsumeOperatorCopyableValuesChecker::check() {
510513
dbgVarInst->getLoc(), SILUndef::get(mvi->getOperand()),
511514
*varInfo, false /*poison*/, UsesMoveableValueDebugInfo);
512515
}
516+
validMoves.push_back(mvi);
513517
}
514518
foundMove = true;
515519
}
@@ -520,6 +524,11 @@ bool ConsumeOperatorCopyableValuesChecker::check() {
520524
if (foundMove) {
521525
dbgVarInst.markAsMoved();
522526
}
527+
528+
if (validMoves.size() > 0) {
529+
canonicalizer.clear();
530+
canonicalizer.canonicalizeValueLifetime(lexicalValue, validMoves);
531+
}
523532
}
524533

525534
return false;
@@ -575,18 +584,16 @@ class ConsumeOperatorCopyableValuesCheckerPass : public SILFunctionTransform {
575584
LLVM_DEBUG(llvm::dbgs() << "*** Checking moved values in fn: "
576585
<< getFunction()->getName() << '\n');
577586

578-
ConsumeOperatorCopyableValuesChecker checker(getFunction());
579-
580-
// If we already had dominance or loop info generated, update them when
581-
// splitting blocks.
582587
auto *dominanceAnalysis = getAnalysis<DominanceAnalysis>();
583-
if (dominanceAnalysis->hasFunctionInfo(fn))
584-
checker.setDominanceToUpdate(dominanceAnalysis->get(fn));
588+
auto *dominance = dominanceAnalysis->get(fn);
589+
auto *calleeAnalysis = getAnalysis<BasicCalleeAnalysis>();
590+
ConsumeOperatorCopyableValuesChecker checker(getFunction(), dominance,
591+
calleeAnalysis);
585592
auto *loopAnalysis = getAnalysis<SILLoopAnalysis>();
586-
if (loopAnalysis->hasFunctionInfo(fn))
587-
checker.setLoopInfoToUpdate(loopAnalysis->get(fn));
588593

589594
if (checker.check()) {
595+
// If we already had dominance or loop info generated, update them when
596+
// splitting blocks.
590597
AnalysisPreserver preserveDominance(dominanceAnalysis);
591598
AnalysisPreserver preserveLoop(loopAnalysis);
592599
invalidateAnalysis(

‎test/Interpreter/consume_operator_kills_copyable_values.swift.gyb

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
// RUN: %target-run-simple-swiftgyb(-Xfrontend -sil-verify-all) | %FileCheck %t/main.swift
2+
// RUN: %target-run-simple-swiftgyb(-O -Xfrontend -sil-verify-all) | %FileCheck %t/main.swift
3+
4+
// This test uses gyb substitute let and var in as the introducers for the
5+
// vardecls that are consumed. This guarantees that the same ordering results
6+
// from checking the consume operator when applied to lets as when applied to
7+
// lets.
8+
9+
// REQUIRES: executable_test
10+
11+
class X {
12+
let number: Int
13+
let function: StaticString
14+
let line: UInt
15+
init(_ number: Int, function: StaticString = #function, line: UInt = #line) {
16+
self.number = number
17+
self.function = function
18+
self.line = line
19+
print("hi - \(function) - \(number) - \(line)")
20+
}
21+
deinit {
22+
print("adios - \(function) - \(number) - \(line)")
23+
}
24+
}
25+
26+
func barrier(_ name: String = "", function: StaticString = #function, line: UInt = #line) {
27+
print("barrier - \(name) - \(function) - \(line)")
28+
}
29+
30+
func endcap(name: String, _ args: [Any], function: StaticString = #function, line: UInt = #line) {
31+
print("\(name) \(function)(", terminator: "")
32+
for (index, arg) in args.enumerated() {
33+
print(arg, terminator: "")
34+
if index != args.count - 1 {
35+
print(", ", terminator: "")
36+
}
37+
}
38+
print(") - \(line)")
39+
}
40+
41+
func begin(_ args: [Any], function: StaticString = #function, line: UInt = #line) {
42+
endcap(name: "begin", args, function: function, line: line)
43+
}
44+
45+
func end(_ args: [Any], function: StaticString = #function, line: UInt = #line) {
46+
endcap(name: "end", args, function: function, line: line)
47+
}
48+
49+
func takeX(_ x: consuming X) {}
50+
51+
% for introducer in ["let", "var"]:
52+
53+
func simple_${introducer}() {
54+
// CHECK-LABEL: begin simple_{{let|var}}
55+
begin([])
56+
do {
57+
// CHECK: hi
58+
${introducer} x = X(0)
59+
// CHECK: adios
60+
takeX(consume x)
61+
// CHECK: barrier
62+
barrier()
63+
}
64+
// CHECK-LABEL: end simple_{{let|var}}
65+
end([])
66+
}
67+
68+
// CHECK-LABEL: begin testLeft_{{let|var}}(_:)(true)
69+
// CHECK: hi
70+
// CHECK: adios
71+
// CHECK: barrier - left
72+
// CHECK: barrier - end
73+
// CHECK-LABEL: end testLeft_{{let|var}}(_:)(true)
74+
// CHECK-LABEL: begin testLeft_{{let|var}}(_:)(false)
75+
// CHECK: hi
76+
// CHECK: barrier - right
77+
// CHECK: adios
78+
// CHECK: barrier - end
79+
// CHECK-LABEL: end testLeft_{{let|var}}(_:)(false)
80+
func testLeft_${introducer}(_ condition: Bool) {
81+
begin([condition])
82+
do {
83+
${introducer} x = X(0)
84+
if condition {
85+
takeX(consume x)
86+
barrier("left")
87+
} else {
88+
barrier("right")
89+
}
90+
barrier("end")
91+
}
92+
end([condition])
93+
}
94+
95+
// CHECK-LABEL: begin testNested1_{{let|var}}(_:_:)(false, false)
96+
// CHECK: hi
97+
// CHECK: barrier - not c1, not c2
98+
// CHECK: barrier - not c1
99+
// CHECK: adios
100+
// CHECK-LABEL: end testNested1_{{let|var}}(_:_:)(false, false)
101+
// CHECK-LABEL: begin testNested1_{{let|var}}(_:_:)(false, true)
102+
// CHECK: hi
103+
// CHECK: barrier - not c1, c2
104+
// CHECK: barrier - not c1
105+
// CHECK: adios
106+
// CHECK-LABEL: end testNested1_{{let|var}}(_:_:)(false, true)
107+
// CHECK-LABEL: begin testNested1_{{let|var}}(_:_:)(true, false)
108+
// CHECK: hi
109+
// CHECK: adios
110+
// CHECK: barrier - c2
111+
// CHECK-LABEL: end testNested1_{{let|var}}(_:_:)(true, false)
112+
// CHECK-LABEL: begin testNested1_{{let|var}}(_:_:)(true, true)
113+
// CHECK: hi
114+
// CHECK: adios
115+
// CHECK: barrier - c2
116+
// CHECK-LABEL: end testNested1_{{let|var}}(_:_:)(true, true)
117+
func testNested1_${introducer}(_ c1: Bool, _ c2: Bool) {
118+
begin([c1, c2])
119+
do {
120+
${introducer} x = X(0)
121+
if c1 {
122+
takeX(consume x)
123+
barrier("c2")
124+
} else {
125+
if c2 {
126+
takeX(x)
127+
barrier("not c1, c2")
128+
} else {
129+
barrier("not c1, not c2")
130+
}
131+
barrier("not c1")
132+
}
133+
}
134+
end([c1, c2])
135+
}
136+
137+
// CHECK-LABEL: begin testNested2_{{let|var}}(_:_:)(false, false)
138+
// CHECK: hi
139+
// CHECK: barrier - not c1, not c2
140+
// CHECK: adios
141+
// CHECK: barrier - not c1
142+
// CHECK-LABEL: end testNested2_{{let|var}}(_:_:)(false, false)
143+
// CHECK-LABEL: begin testNested2_{{let|var}}(_:_:)(false, true)
144+
// CHECK: hi
145+
// CHECK: adios
146+
// CHECK: barrier - not c1, c2
147+
// CHECK: barrier - not c1
148+
// CHECK-LABEL: end testNested2_{{let|var}}(_:_:)(false, true)
149+
// CHECK-LABEL: begin testNested2_{{let|var}}(_:_:)(true, false)
150+
// CHECK: hi
151+
// CHECK: adios
152+
// CHECK: barrier - c1
153+
// CHECK-LABEL: end testNested2_{{let|var}}(_:_:)(true, false)
154+
// CHECK-LABEL: begin testNested2_{{let|var}}(_:_:)(true, true)
155+
// CHECK: hi
156+
// CHECK: adios
157+
// CHECK: barrier - c1
158+
// CHECK-LABEL: end testNested2_{{let|var}}(_:_:)(true, true)
159+
func testNested2_${introducer}(_ c1: Bool, _ c2: Bool) {
160+
begin([c1, c2])
161+
do {
162+
${introducer} x = X(0)
163+
if c1 {
164+
takeX(consume x)
165+
barrier("c1")
166+
} else {
167+
if c2 {
168+
takeX(consume x)
169+
barrier("not c1, c2")
170+
} else {
171+
barrier("not c1, not c2")
172+
}
173+
barrier("not c1")
174+
}
175+
}
176+
end([c1, c2])
177+
}
178+
179+
func main_${introducer}() {
180+
simple_${introducer}()
181+
testLeft_${introducer}(true)
182+
testLeft_${introducer}(false)
183+
testNested1_${introducer}(false, false)
184+
testNested1_${introducer}(false, true)
185+
testNested1_${introducer}(true, false)
186+
testNested1_${introducer}(true, true)
187+
testNested2_${introducer}(false, false)
188+
testNested2_${introducer}(false, true)
189+
testNested2_${introducer}(true, false)
190+
testNested2_${introducer}(true, true)
191+
}
192+
193+
% end
194+
195+
main_let()
196+
main_var()

‎validation-test/SILOptimizer/rdar113142446.sil

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %target-sil-opt -opt-mode=none -enable-sil-verify-all %s -sil-consume-operator-copyable-values-checker | %FileCheck %s
2+
// RUN: %target-sil-opt -opt-mode=speed -enable-sil-verify-all %s -sil-consume-operator-copyable-values-checker | %FileCheck %s
3+
4+
sil_stage raw
5+
6+
import Builtin
7+
8+
class X {}
9+
10+
struct S {}
11+
12+
sil @getX : $@convention(thin) () -> @owned X
13+
sil @takeX : $@convention(thin) (@owned X) -> S
14+
15+
// CHECK-LABEL: sil [ossa] @testit : {{.*}} {
16+
// CHECK-NOT: copy_value
17+
// CHECK-LABEL: } // end sil function 'testit'
18+
sil [ossa] @testit : $@convention(thin) () -> () {
19+
%getX = function_ref @getX : $@convention(thin) () -> @owned X
20+
%x = apply %getX() : $@convention(thin) () -> @owned X
21+
%begin = move_value [lexical] [var_decl] %x : $X
22+
debug_value %begin : $X, let, name "x"
23+
%copy = copy_value %begin : $X
24+
%consume = move_value [allows_diagnostics] %copy : $X
25+
%takeX = function_ref @takeX : $@convention(thin) (@owned X) -> S
26+
apply %takeX(%consume) : $@convention(thin) (@owned X) -> S
27+
destroy_value %begin : $X
28+
%retval = tuple ()
29+
return %retval : $()
30+
}
31+

0 commit comments

Comments
 (0)