Skip to content

Commit 3795589

Browse files
committed
[move-function] Add support for properly checking let x: Int without an initial value.
We process these as loadable vars. This is really useful since it ensures that uniqueness is preserved in this case: ``` let x: K2 do { x = self.k2 } switch _move(x)[userHandle] { case .foo: assert(_isUnique(&self.k2)) } ``` I added a test that proves this. (cherry picked from commit 396f510)
1 parent 2f5ee16 commit 3795589

File tree

5 files changed

+147
-7
lines changed

5 files changed

+147
-7
lines changed

lib/SILOptimizer/Mandatory/MoveFunctionCanonicalization.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,10 @@ tryHandlingLoadableVarMovePattern(MarkUnresolvedMoveAddrInst *markMoveAddr,
113113
LLVM_DEBUG(llvm::dbgs() << "Found LI: " << *li);
114114
SILValue operand = stripAccessMarkers(li->getOperand());
115115
auto *originalASI = dyn_cast<AllocStackInst>(operand);
116-
if (!originalASI || !originalASI->isLexical() || !originalASI->isVar())
116+
// Make sure that we have a lexical alloc_stack marked as a var or a let. We
117+
// don't want properties.
118+
if (!originalASI || !originalASI->isLexical() ||
119+
!(originalASI->isVar() || originalASI->isLet()))
117120
return false;
118121

119122
LLVM_DEBUG(llvm::dbgs() << "Found OriginalASI: " << *originalASI);

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ static llvm::cl::opt<bool>
162162
DisableUnhandledMoveDiagnostic("sil-disable-unknown-moveaddr-diagnostic");
163163

164164
//===----------------------------------------------------------------------===//
165-
// Diagnostic Utilities
165+
// Utilities
166166
//===----------------------------------------------------------------------===//
167167

168168
template <typename... T, typename... U>
@@ -171,6 +171,15 @@ static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag,
171171
Context.Diags.diagnose(loc, diag, std::forward<U>(args)...);
172172
}
173173

174+
namespace llvm {
175+
llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const SmallBitVector &bv) {
176+
for (unsigned index : range(bv.size())) {
177+
os << (bv[index] ? 1 : 0);
178+
}
179+
return os;
180+
}
181+
} // namespace llvm
182+
174183
//===----------------------------------------------------------------------===//
175184
// Use Gathering
176185
//===----------------------------------------------------------------------===//
@@ -610,29 +619,36 @@ cleanupAllDestroyAddr(SILFunction *fn, SmallBitVector &destroyIndices,
610619
bool madeChange = false;
611620
BasicBlockWorklist worklist(fn);
612621
SILValue daiOperand;
622+
LLVM_DEBUG(llvm::dbgs() << "Cleanup up destroy addr!\n");
623+
LLVM_DEBUG(llvm::dbgs() << " Visiting destroys!\n");
624+
LLVM_DEBUG(llvm::dbgs() << " Destroy Indices: " << destroyIndices << "\n");
613625
for (int index = destroyIndices.find_first(); index != -1;
614626
index = destroyIndices.find_next(index)) {
627+
LLVM_DEBUG(llvm::dbgs() << " Index: " << index << "\n");
615628
auto dai = useState.destroys[index];
616629
if (!dai)
617630
continue;
631+
LLVM_DEBUG(llvm::dbgs() << " Destroy: " << *dai);
618632
SILValue op = (*dai)->getOperand();
619633
assert(daiOperand == SILValue() || op == daiOperand);
620634
daiOperand = op;
621635
for (auto *predBlock : (*dai)->getParent()->getPredecessorBlocks()) {
622636
worklist.pushIfNotVisited(predBlock);
623637
}
624638
}
625-
639+
LLVM_DEBUG(llvm::dbgs() << " Visiting reinit!\n");
626640
for (int index = reinitIndices.find_first(); index != -1;
627641
index = reinitIndices.find_next(index)) {
628642
auto reinit = useState.reinits[index];
629643
if (!reinit)
630644
continue;
645+
LLVM_DEBUG(llvm::dbgs() << " Reinit: " << *reinit);
631646
for (auto *predBlock : (*reinit)->getParent()->getPredecessorBlocks()) {
632647
worklist.pushIfNotVisited(predBlock);
633648
}
634649
}
635650

651+
LLVM_DEBUG(llvm::dbgs() << "Processing worklist!\n");
636652
while (auto *next = worklist.pop()) {
637653
LLVM_DEBUG(llvm::dbgs()
638654
<< "Looking at block: bb" << next->getDebugID() << "\n");
@@ -699,10 +715,6 @@ bool MoveKillsCopyableAddressesObjectChecker::performGlobalDataflow(
699715
BasicBlockSet blocksWithMovesThatAreNowTakes(fn);
700716
bool convertedMarkMoveToTake = false;
701717
for (auto *mvi : markMovesToDataflow) {
702-
// At the end of this if we already initialized indices of paired destroys,
703-
// clear it.
704-
SWIFT_DEFER { getIndicesOfPairedDestroys().clear(); };
705-
706718
bool emittedSingleDiagnostic = false;
707719

708720
LLVM_DEBUG(llvm::dbgs() << "Checking Multi Block Dataflow for: " << *mvi);

test/SILOptimizer/move_function_kills_copyable_loadable_vars.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,23 @@ public func performMoveOnVarMultiBlockError2(_ p: Klass) {
9393
x = p
9494
nonConsumingUse(x)
9595
}
96+
97+
// Even though the examples below are for lets, since the let is not initially
98+
// defined it comes out like a var.
99+
public func performMoveOnLaterDefinedInit(_ p: Klass) {
100+
let x: Klass // expected-error {{'x' used after being moved}}
101+
do {
102+
x = p
103+
}
104+
let _ = _move(x) // expected-note {{move here}}
105+
nonConsumingUse(x) // expected-note {{use here}}
106+
}
107+
108+
public func performMoveOnLaterDefinedInit2(_ p: Klass) {
109+
let x: Klass
110+
do {
111+
x = p
112+
}
113+
nonConsumingUse(x)
114+
let _ = _move(x)
115+
}

test/lit.cfg

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,7 @@ def use_interpreter_for_simple_runs():
901901
config.target_run_stdlib_swift = make_simple_target_run(stdlib=True)
902902
config.target_run_simple_swift = make_simple_target_run()
903903
config.target_run_simple_swift_parameterized = make_simple_target_run(parameterized=True)
904+
config.target_run_stdlib_swift_parameterized = make_simple_target_run(stdlib=True, parameterized=True)
904905
config.target_run_simple_swiftgyb_parameterized = make_simple_target_run(gyb=True, parameterized=True)
905906
config.available_features.add('interpret')
906907

@@ -2077,6 +2078,17 @@ if not getattr(config, 'target_run_simple_swift', None):
20772078
escape_for_substitute_captures(config.target_codesign),
20782079
escape_for_substitute_captures(config.target_run))
20792080
)
2081+
config.target_run_stdlib_swift_parameterized = SubstituteCaptures(
2082+
r"%%empty-directory(%%t) && "
2083+
r"%s %s %%s \1 -o %%t/a.out -module-name main "
2084+
r" -Xfrontend -disable-access-control && "
2085+
r"%s %%t/a.out && "
2086+
r"%s %%t/a.out"
2087+
% (escape_for_substitute_captures(config.target_build_swift),
2088+
escape_for_substitute_captures(mcp_opt),
2089+
escape_for_substitute_captures(config.target_codesign),
2090+
escape_for_substitute_captures(config.target_run))
2091+
)
20802092

20812093
config.target_run_simple_swift = (
20822094
'%%empty-directory(%%t) && '
@@ -2167,6 +2179,9 @@ config.substitutions.append(('%target-run-simple-swift\(([^)]+)\)',
21672179
config.target_run_simple_swift_parameterized))
21682180
config.substitutions.append(('%target-fail-simple-swift\(([^)]+)\)',
21692181
config.target_fail_simple_swift_parameterized))
2182+
config.substitutions.append(('%target-run-stdlib-swift\(([^)]+)\)',
2183+
config.target_run_stdlib_swift_parameterized))
2184+
21702185
config.substitutions.append(('%target-run-simple-swift', config.target_run_simple_swift))
21712186
config.substitutions.append(('%target-run-stdlib-swiftgyb', config.target_run_stdlib_swiftgyb))
21722187
config.substitutions.append(('%target-run-stdlib-swift', config.target_run_stdlib_swift))

test/stdlib/move_function.swift

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// RUN: %target-run-stdlib-swift(-Xllvm -sil-disable-pass=DestroyHoisting)
2+
//
3+
// NOTE ON ABOVE: I am disabling destroy hoisting on this test since we are
4+
// going to move it out of the mandatory pipeline eventually and it causes the
5+
// test to fail only when optimizations are enabled.
6+
7+
// REQUIRES: executable_test
8+
9+
import Swift
10+
import StdlibUnittest
11+
12+
public class Klass {}
13+
14+
var tests = TestSuite("move_function_uniqueness")
15+
16+
public enum Enum {
17+
case foo
18+
}
19+
20+
public class K2 {
21+
var array: [Enum] = Array(repeating: .foo, count: 10_000)
22+
23+
subscript(index: Int) -> Enum {
24+
@inline(never)
25+
get {
26+
array[index]
27+
}
28+
@inline(never)
29+
set {
30+
array[index] = newValue
31+
}
32+
@inline(never)
33+
_modify {
34+
yield &array[index]
35+
}
36+
}
37+
}
38+
39+
public class Class {
40+
var k2 = K2()
41+
var array: [Enum] = Array(repeating: .foo, count: 10_000)
42+
}
43+
44+
extension Class {
45+
@inline(never)
46+
func readClassTest(_ userHandle: Int) {
47+
assert(_isUnique(&self.k2))
48+
49+
let x: K2
50+
do {
51+
x = self.k2
52+
}
53+
switch _move(x)[userHandle] {
54+
case .foo:
55+
assert(_isUnique(&self.k2))
56+
}
57+
}
58+
}
59+
60+
tests.test("classUniquenessTest") {
61+
let c = Class()
62+
for f in 0..<10_000 {
63+
c.readClassTest(f)
64+
}
65+
}
66+
67+
extension Class {
68+
@inline(never)
69+
func readArrayTest(_ userHandle: Int) {
70+
assert(self.array._buffer.isUniquelyReferenced())
71+
72+
let x: [Enum]
73+
do {
74+
x = self.array
75+
}
76+
switch _move(x)[userHandle] {
77+
case .foo:
78+
assert(self.array._buffer.isUniquelyReferenced())
79+
}
80+
}
81+
}
82+
83+
tests.test("arrayUniquenessTest") {
84+
let c = Class()
85+
for f in 0..<10_000 {
86+
c.readArrayTest(f)
87+
}
88+
}
89+
90+
runAllTests()

0 commit comments

Comments
 (0)