Skip to content

Commit 544b338

Browse files
committed
[move-function] Add support for loadable vars.
This is done by extending the support in the previous PR for a. If we see the following pattern: ``` %0 = alloc_stack [lexical] $LoadableType, var ... *init of %0* ... %1 = load [copy] %0 : $*LoadableType %temporary = alloc_stack $LoadableType store %1 to [init] %temporary : $*LoadableType mark_unresolved_move_addr %temporary to %otherAddr : $*LoadableType destroy_addr %temporary : $*LoadableType ``` we transform it to: ``` %0 = alloc_stack [lexical] $LoadableType, var ... *init of %0* ... %1 = load [copy] %0 : $*LoadableType %temporary = alloc_stack $LoadableType store %2 to [init] %temporary : $*LoadableType mark_unresolved_move_addr %0 to %otherAddr : $*LoadableType destroy_addr %temporary : $*LoadableType ``` For safety reasons, we always make sure that %0 isn't destroyed in between the load [copy] and the mark_unresolved_move_addr. After this runs, we can use the address verifier on these types of vars with some additional work that I am doing in a subsequent commit.
1 parent 4a9c26f commit 544b338

File tree

3 files changed

+192
-31
lines changed

3 files changed

+192
-31
lines changed

lib/SILOptimizer/Mandatory/MoveFunctionCanonicalization.cpp

Lines changed: 97 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,23 +39,9 @@ using namespace swift;
3939
// Utility
4040
//===----------------------------------------------------------------------===//
4141

42-
/// Attempts to perform several small optimizations to setup both the address
43-
/// and object checkers. Returns true if we made a change to the IR.
44-
static bool tryConvertSimpleMoveFromAllocStackTemporary(
45-
MarkUnresolvedMoveAddrInst *markMoveAddr, AliasAnalysis *aa) {
46-
LLVM_DEBUG(llvm::dbgs() << "Trying to fix up: " << *markMoveAddr);
47-
48-
// We need a non-lexical alloc_stack as our source.
49-
auto *asi = dyn_cast<AllocStackInst>(markMoveAddr->getSrc());
50-
if (!asi || asi->isLexical()) {
51-
LLVM_DEBUG(llvm::dbgs()
52-
<< " Source isnt an alloc_stack or is lexical... Bailing!\n");
53-
return false;
54-
}
55-
56-
DestroyAddrInst *dai = nullptr;
57-
CopyAddrInst *cai = nullptr;
58-
StoreInst *si = nullptr;
42+
static bool findInitAndDestroyForAllocation(
43+
AllocStackInst *asi, MarkUnresolvedMoveAddrInst *markMoveAddr,
44+
CopyAddrInst *&cai, DestroyAddrInst *&dai, StoreInst *&si) {
5945
for (auto *use : asi->getUses()) {
6046
auto *user = use->getUser();
6147
LLVM_DEBUG(llvm::dbgs() << " Visiting User: " << *user);
@@ -113,6 +99,86 @@ static bool tryConvertSimpleMoveFromAllocStackTemporary(
11399
return false;
114100
}
115101

102+
return true;
103+
}
104+
105+
static bool
106+
tryHandlingLoadableVarMovePattern(MarkUnresolvedMoveAddrInst *markMoveAddr,
107+
StoreInst *si, AliasAnalysis *aa) {
108+
auto *li = dyn_cast<LoadInst>(si->getSrc());
109+
if (!li || li->getOwnershipQualifier() != LoadOwnershipQualifier::Copy ||
110+
li->getParent() != si->getParent())
111+
return false;
112+
113+
LLVM_DEBUG(llvm::dbgs() << "Found LI: " << *li);
114+
SILValue operand = stripAccessMarkers(li->getOperand());
115+
auto *originalASI = dyn_cast<AllocStackInst>(operand);
116+
if (!originalASI || !originalASI->isLexical() || !originalASI->isVar())
117+
return false;
118+
119+
LLVM_DEBUG(llvm::dbgs() << "Found OriginalASI: " << *originalASI);
120+
// Make sure that there aren't any side-effect having instructions in
121+
// between our load/store.
122+
LLVM_DEBUG(llvm::dbgs() << "Checking for uses in between LI and SI.\n");
123+
auto range =
124+
llvm::make_range(std::next(li->getIterator()), si->getIterator());
125+
if (!llvm::none_of(range, [&](SILInstruction &iter) {
126+
if (!iter.mayHaveSideEffects()) {
127+
LLVM_DEBUG(llvm::dbgs() << "Found no side effect inst: " << iter);
128+
return false;
129+
}
130+
131+
if (auto *dvi = dyn_cast<DestroyAddrInst>(&iter)) {
132+
if (aa->isNoAlias(dvi->getOperand(), originalASI)) {
133+
// We are going to be extending the lifetime of our
134+
// underlying value, not shrinking it so we can ignore
135+
// destroy_addr on other non-aliasing values.
136+
LLVM_DEBUG(llvm::dbgs() << "Found no alias destroy_addr: " << iter);
137+
return false;
138+
}
139+
}
140+
141+
// Ignore end of scope markers with side-effects.
142+
if (isEndOfScopeMarker(&iter)) {
143+
LLVM_DEBUG(llvm::dbgs() << "Found end of scope marker: " << iter);
144+
return false;
145+
}
146+
147+
LLVM_DEBUG(llvm::dbgs()
148+
<< " Found side-effect inst... Bailing!: " << iter);
149+
return true;
150+
})) {
151+
return false;
152+
}
153+
154+
// Ok, we know our original lexical alloc_stack is not written to in between
155+
// the load/store. Move the mark_move_addr onto the lexical alloc_stack.
156+
LLVM_DEBUG(llvm::dbgs() << " Doing loadable var!\n");
157+
markMoveAddr->setSrc(originalASI);
158+
return true;
159+
}
160+
161+
/// Attempts to perform several small optimizations to setup both the address
162+
/// and object checkers. Returns true if we made a change to the IR.
163+
static bool tryConvertSimpleMoveFromAllocStackTemporary(
164+
MarkUnresolvedMoveAddrInst *markMoveAddr, AliasAnalysis *aa,
165+
InstructionDeleter &deleter) {
166+
LLVM_DEBUG(llvm::dbgs() << "Trying to fix up: " << *markMoveAddr);
167+
168+
// We need a non-lexical alloc_stack as our source.
169+
auto *asi = dyn_cast<AllocStackInst>(markMoveAddr->getSrc());
170+
if (!asi || asi->isLexical()) {
171+
LLVM_DEBUG(llvm::dbgs()
172+
<< " Source isnt an alloc_stack or is lexical... Bailing!\n");
173+
return false;
174+
}
175+
176+
DestroyAddrInst *dai = nullptr;
177+
CopyAddrInst *cai = nullptr;
178+
StoreInst *si = nullptr;
179+
if (!findInitAndDestroyForAllocation(asi, markMoveAddr, cai, dai, si))
180+
return false;
181+
116182
// If we did not find an (init | store) or destroy_addr, just bail.
117183
if (!(cai || si) || !dai) {
118184
LLVM_DEBUG(llvm::dbgs()
@@ -122,7 +188,7 @@ static bool tryConvertSimpleMoveFromAllocStackTemporary(
122188

123189
assert(bool(cai) != bool(si));
124190

125-
// Otherwise, lets walk from cai to markMoveAddr and make sure there aren't
191+
// Otherwise, lets walk from cai/si to markMoveAddr and make sure there aren't
126192
// any side-effect having instructions in between them.
127193
//
128194
// NOTE: We know that cai must be before the markMoveAddr in the block since
@@ -160,11 +226,11 @@ static bool tryConvertSimpleMoveFromAllocStackTemporary(
160226
}))
161227
return false;
162228

163-
LLVM_DEBUG(llvm::dbgs() << " Success! Performing optimization!\n");
164229
// Ok, we can perform our optimization! Change move_addr's source to be the
165230
// original copy_addr's src and add add uses of the stack location to an
166231
// instruction deleter. We will eliminate them later.
167232
if (cai) {
233+
LLVM_DEBUG(llvm::dbgs() << " Success! Performing optimization!\n");
168234
markMoveAddr->setSrc(cai->getSrc());
169235
return true;
170236
}
@@ -177,8 +243,8 @@ static bool tryConvertSimpleMoveFromAllocStackTemporary(
177243
// re-materialized into an alloc_stack. In this example remembering that
178244
// mark_unresolved_move_addr is a copy_addr [init], we try to move the MUMA
179245
// onto the original lexical alloc_stack.
180-
//
181-
// TODO: Implement.
246+
if (tryHandlingLoadableVarMovePattern(markMoveAddr, si, aa))
247+
return true;
182248

183249
// If we do not have a load [copy], transform this mark_resolved_move_addr
184250
// into a move_value [diagnostic] + store [init]. Predictable mem opts is
@@ -188,7 +254,11 @@ static bool tryConvertSimpleMoveFromAllocStackTemporary(
188254
auto *newValue = builder.createMoveValue(si->getLoc(), si->getSrc());
189255
newValue->setAllowsDiagnostics(true);
190256
si->setSrc(newValue);
191-
markMoveAddr->eraseFromParent();
257+
si->setDest(markMoveAddr->getDest());
258+
deleter.forceTrackAsDead(markMoveAddr);
259+
deleter.forceTrackAsDead(dai);
260+
261+
LLVM_DEBUG(llvm::dbgs() << " Success! Performing optimization!\n");
192262
return true;
193263
}
194264

@@ -217,6 +287,7 @@ class MoveFunctionCanonicalization : public SILFunctionTransform {
217287
"Should only run on Raw SIL");
218288

219289
auto *aa = getAnalysis<AliasAnalysis>(fn);
290+
InstructionDeleter deleter;
220291

221292
for (auto &block : *fn) {
222293
for (auto ii = block.begin(), ie = block.end(); ii != ie;) {
@@ -228,13 +299,15 @@ class MoveFunctionCanonicalization : public SILFunctionTransform {
228299
// the mark_unresolved_move_addr is always on the operand regardless if
229300
// in the caller we materalized the address into a temporary.
230301
if (auto *markMoveAddr = dyn_cast<MarkUnresolvedMoveAddrInst>(inst)) {
231-
madeChange |=
232-
tryConvertSimpleMoveFromAllocStackTemporary(markMoveAddr, aa);
302+
madeChange |= tryConvertSimpleMoveFromAllocStackTemporary(
303+
markMoveAddr, aa, deleter);
233304
continue;
234305
}
235306
}
236307
}
237308

309+
deleter.cleanupDeadInstructions();
310+
238311
if (madeChange) {
239312
invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
240313
}
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// RUN: %target-swift-frontend -enable-experimental-move-only -verify %s -parse-stdlib -emit-sil -o /dev/null
2+
3+
// REQUIRES: optimized_stdlib
4+
5+
import Swift
6+
7+
public class Klass {}
8+
9+
//////////////////
10+
// Declarations //
11+
//////////////////
12+
13+
func consumingUse(_ k: __owned Klass) {}
14+
var booleanValue: Bool { false }
15+
func nonConsumingUse(_ k: Klass) {}
16+
17+
///////////
18+
// Klassests //
19+
///////////
20+
21+
public func performMoveOnVarSingleBlock(_ p: Klass) {
22+
var x = p
23+
let _ = _move(x)
24+
x = p
25+
nonConsumingUse(x)
26+
}
27+
28+
public func performMoveOnVarSingleBlockError(_ p: Klass) {
29+
var x = p // expected-error {{'x' used after being moved}}
30+
let _ = _move(x) // expected-note {{move here}}
31+
nonConsumingUse(x) // expected-note {{use here}}
32+
x = p
33+
nonConsumingUse(x)
34+
}
35+
36+
public func performMoveOnVarMultiBlock(_ p: Klass) {
37+
var x = p
38+
let _ = _move(x)
39+
40+
while booleanValue {
41+
print("true")
42+
}
43+
44+
while booleanValue {
45+
print("true")
46+
}
47+
48+
x = p
49+
nonConsumingUse(x)
50+
}
51+
52+
public func performMoveOnVarMultiBlockError1(_ p: Klass) {
53+
var x = p // expected-error {{'x' used after being moved}}
54+
let _ = _move(x) // expected-note {{move here}}
55+
56+
nonConsumingUse(x) // expected-note {{use here}}
57+
58+
while booleanValue {
59+
print("true")
60+
}
61+
62+
// We only emit an error on the first one.
63+
nonConsumingUse(x)
64+
65+
while booleanValue {
66+
print("true")
67+
}
68+
69+
// We only emit an error on the first one.
70+
nonConsumingUse(x)
71+
72+
x = p
73+
nonConsumingUse(x)
74+
}
75+
76+
public func performMoveOnVarMultiBlockError2(_ p: Klass) {
77+
var x = p // expected-error {{'x' used after being moved}}
78+
let _ = _move(x) // expected-note {{move here}}
79+
80+
while booleanValue {
81+
print("true")
82+
}
83+
84+
nonConsumingUse(x) // expected-note {{use here}}
85+
86+
while booleanValue {
87+
print("true")
88+
}
89+
90+
// We only error on the first one.
91+
nonConsumingUse(x)
92+
93+
x = p
94+
nonConsumingUse(x)
95+
}

test/SILOptimizer/move_function_kills_copyable_values.swift

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -245,13 +245,6 @@ public func performMoveOnVarGlobalError() {
245245
let _ = _move(myVarGlobal) // expected-error {{_move applied to value that the compiler does not support checking}}
246246
}
247247

248-
// TODO: Support vars
249-
public func performMoveOnVarError() {
250-
var k = myVarGlobal
251-
let _ = _move(k) // expected-error {{_move applied to value that the compiler does not support checking}}
252-
k = myVarGlobal
253-
}
254-
255248
public func performMoveOnLetGlobalError() {
256249
let _ = _move(myVarGlobal) // expected-error {{_move applied to value that the compiler does not support checking}}
257250
}

0 commit comments

Comments
 (0)