Skip to content

Commit a734a24

Browse files
gottesmmgottesmm
committed
[addr-move-function] Make sure that we handle closure arguments in multi-block callers.
I just forgot to include this in the previous PR. I added some more tests around it.
1 parent 4ae07e6 commit a734a24

File tree

3 files changed

+210
-4
lines changed

3 files changed

+210
-4
lines changed

‎lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -731,12 +731,14 @@ void ClosureArgDataflowState::classifyUses(BasicBlockSet &initBlocks,
731731

732732
for (auto *user : useState.inits) {
733733
if (upwardScanForInit(user, useState)) {
734+
LLVM_DEBUG(llvm::dbgs() << " Found init block at: " << *user);
734735
initBlocks.insert(user->getParent());
735736
}
736737
}
737738

738739
for (auto *user : useState.livenessUses) {
739740
if (upwardScanForUseOut(user, useState)) {
741+
LLVM_DEBUG(llvm::dbgs() << " Found use block at: " << *user);
740742
livenessBlocks.insert(user->getParent());
741743
livenessWorklist.push_back(user);
742744
}
@@ -1338,7 +1340,8 @@ struct DataflowState {
13381340
llvm::DenseSet<SILBasicBlock *> initBlocks;
13391341
llvm::DenseMap<SILBasicBlock *, SILInstruction *> destroyBlocks;
13401342
llvm::DenseMap<SILBasicBlock *, SILInstruction *> reinitBlocks;
1341-
llvm::DenseMap<SILBasicBlock *, Operand *> consumingClosureBlocks;
1343+
llvm::DenseMap<SILBasicBlock *, Operand *> closureConsumeBlocks;
1344+
llvm::DenseMap<SILBasicBlock *, ClosureOperandState *> closureUseBlocks;
13421345
SmallVector<MarkUnresolvedMoveAddrInst *, 8> markMovesThatPropagateDownwards;
13431346

13441347
SILOptFunctionBuilder &funcBuilder;
@@ -1372,7 +1375,8 @@ struct DataflowState {
13721375
destroyBlocks.clear();
13731376
reinitBlocks.clear();
13741377
markMovesThatPropagateDownwards.clear();
1375-
consumingClosureBlocks.clear();
1378+
closureConsumeBlocks.clear();
1379+
closureUseBlocks.clear();
13761380
}
13771381
};
13781382

@@ -1586,6 +1590,38 @@ bool DataflowState::process(
15861590
break;
15871591
}
15881592

1593+
// Now see if we have a closure use.
1594+
{
1595+
auto iter = closureUseBlocks.find(next);
1596+
if (iter != closureUseBlocks.end()) {
1597+
LLVM_DEBUG(llvm::dbgs() << " Is Use Block! Emitting Error!\n");
1598+
// We found one! Emit the diagnostic and continue and see if we can
1599+
// get more diagnostics.
1600+
auto &astContext = fn->getASTContext();
1601+
{
1602+
auto diag =
1603+
diag::sil_movekillscopyablevalue_value_consumed_more_than_once;
1604+
StringRef name = getDebugVarName(address);
1605+
diagnose(astContext, getSourceLocFromValue(address), diag, name);
1606+
}
1607+
1608+
{
1609+
auto diag = diag::sil_movekillscopyablevalue_move_here;
1610+
diagnose(astContext, mvi->getLoc().getSourceLoc(), diag);
1611+
}
1612+
1613+
{
1614+
auto diag = diag::sil_movekillscopyablevalue_use_here;
1615+
for (auto *user : iter->second->pairedUseInsts) {
1616+
diagnose(astContext, user->getLoc().getSourceLoc(), diag);
1617+
}
1618+
}
1619+
1620+
emittedSingleDiagnostic = true;
1621+
break;
1622+
}
1623+
}
1624+
15891625
// Then see if this is a destroy block. If so, do not add successors and
15901626
// continue. This is because we stop processing at destroy_addr. This
15911627
// destroy_addr is paired with the mark_unresolved_move_addr.
@@ -1614,8 +1650,8 @@ bool DataflowState::process(
16141650
}
16151651

16161652
{
1617-
auto iter = consumingClosureBlocks.find(next);
1618-
if (iter != consumingClosureBlocks.end()) {
1653+
auto iter = closureConsumeBlocks.find(next);
1654+
if (iter != closureConsumeBlocks.end()) {
16191655
LLVM_DEBUG(llvm::dbgs() << " Is reinit Block! Setting up for "
16201656
"later deletion if possible!\n");
16211657
auto indexIter = useState.closureOperandToIndexMap.find(iter->second);
@@ -1737,6 +1773,35 @@ void DataflowState::init() {
17371773
reinitBlocks[reinit->getParent()] = reinit;
17381774
}
17391775
}
1776+
1777+
for (auto closureUse : useState.closureUses) {
1778+
auto *use = closureUse.first;
1779+
auto &state = closureUse.second;
1780+
auto *user = use->getUser();
1781+
1782+
switch (state.result) {
1783+
case DownwardScanResult::Invalid:
1784+
case DownwardScanResult::Destroy:
1785+
case DownwardScanResult::Reinit:
1786+
case DownwardScanResult::UseForDiagnostic:
1787+
case DownwardScanResult::MoveOut:
1788+
llvm_unreachable("unhandled");
1789+
case DownwardScanResult::ClosureUse:
1790+
if (upwardScanForUseOut(user, useState)) {
1791+
LLVM_DEBUG(llvm::dbgs()
1792+
<< " Found closure liveness block at: " << *user);
1793+
closureUseBlocks[user->getParent()] = &state;
1794+
}
1795+
break;
1796+
case DownwardScanResult::ClosureConsume:
1797+
if (upwardScanForDestroys(user, useState)) {
1798+
LLVM_DEBUG(llvm::dbgs()
1799+
<< " Found closure consuming block at: " << *user);
1800+
closureConsumeBlocks[user->getParent()] = use;
1801+
}
1802+
break;
1803+
}
1804+
}
17401805
}
17411806

17421807
// Returns true if we emitted a diagnostic and handled the single block

‎test/SILOptimizer/move_function_kills_copyable_addressonly_vars.swift

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,79 @@ extension DeferTestProtocol {
346346
}
347347
print("123")
348348
}
349+
350+
mutating func deferTestFail8() { // expected-error {{'self' used after being moved}}
351+
let selfType = type(of: self)
352+
let _ = _move(self) // expected-note {{move here}}
353+
defer {
354+
if booleanValue {
355+
nonConsumingUse(k) // expected-note {{use here}}
356+
}
357+
self = selfType.getP()
358+
}
359+
print("foo bar")
360+
}
361+
362+
mutating func deferTestFail9() { // expected-error {{'self' used after being moved}}
363+
let selfType = type(of: self)
364+
let _ = _move(self) // expected-note {{move here}}
365+
defer {
366+
if booleanValue {
367+
nonConsumingUse(k) // expected-note {{use here}}
368+
} else {
369+
nonConsumingUse(k)
370+
}
371+
self = selfType.getP()
372+
}
373+
print("foo bar")
374+
}
375+
376+
mutating func deferTestFail10() { // expected-error {{'self' used after being moved}}
377+
let selfType = type(of: self)
378+
let _ = _move(self) // expected-note {{move here}}
379+
defer {
380+
for _ in 0..<1024 {
381+
nonConsumingUse(k) // expected-note {{use here}}
382+
}
383+
self = selfType.getP()
384+
}
385+
print("foo bar")
386+
}
387+
388+
mutating func deferTestFail11() { // expected-error {{'self' used after being moved}}
389+
let selfType = type(of: self)
390+
let _ = _move(self) // expected-note {{move here}}
391+
if booleanValue {
392+
print("creating blocks")
393+
} else {
394+
print("creating blocks2")
395+
}
396+
defer {
397+
for _ in 0..<1024 {
398+
nonConsumingUse(k) // expected-note {{use here}}
399+
}
400+
self = selfType.getP()
401+
}
402+
print("foo bar")
403+
}
404+
405+
mutating func deferTestFail12() { // expected-error {{'self' used after being moved}}
406+
let selfType = type(of: self)
407+
if booleanValue {
408+
print("creating blocks")
409+
} else {
410+
let _ = _move(self) // expected-note {{move here}}
411+
print("creating blocks2")
412+
}
413+
414+
defer {
415+
for _ in 0..<1024 {
416+
nonConsumingUse(k) // expected-note {{use here}}
417+
}
418+
self = selfType.getP()
419+
}
420+
print("foo bar")
421+
}
349422
}
350423

351424
////////////////

‎test/SILOptimizer/move_function_kills_copyable_loadable_vars.swift

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,74 @@ extension KlassWrapper {
391391
}
392392
print("123")
393393
}
394+
395+
mutating func deferTestFail8() { // expected-error {{'self' used after being moved}}
396+
let _ = _move(self) // expected-note {{move here}}
397+
defer {
398+
if booleanValue {
399+
nonConsumingUse(k) // expected-note {{use here}}
400+
}
401+
self = KlassWrapper(k: Klass())
402+
}
403+
print("foo bar")
404+
}
405+
406+
mutating func deferTestFail9() { // expected-error {{'self' used after being moved}}
407+
let _ = _move(self) // expected-note {{move here}}
408+
defer {
409+
if booleanValue {
410+
nonConsumingUse(k) // expected-note {{use here}}
411+
} else {
412+
nonConsumingUse(k)
413+
}
414+
self = KlassWrapper(k: Klass())
415+
}
416+
print("foo bar")
417+
}
418+
419+
mutating func deferTestFail10() { // expected-error {{'self' used after being moved}}
420+
let _ = _move(self) // expected-note {{move here}}
421+
defer {
422+
for _ in 0..<1024 {
423+
nonConsumingUse(k) // expected-note {{use here}}
424+
}
425+
self = KlassWrapper(k: Klass())
426+
}
427+
print("foo bar")
428+
}
429+
430+
mutating func deferTestFail11() { // expected-error {{'self' used after being moved}}
431+
let _ = _move(self) // expected-note {{move here}}
432+
if booleanValue {
433+
print("creating blocks")
434+
} else {
435+
print("creating blocks2")
436+
}
437+
defer {
438+
for _ in 0..<1024 {
439+
nonConsumingUse(k) // expected-note {{use here}}
440+
}
441+
self = KlassWrapper(k: Klass())
442+
}
443+
print("foo bar")
444+
}
445+
446+
mutating func deferTestFail12() { // expected-error {{'self' used after being moved}}
447+
if booleanValue {
448+
print("creating blocks")
449+
} else {
450+
let _ = _move(self) // expected-note {{move here}}
451+
print("creating blocks2")
452+
}
453+
454+
defer {
455+
for _ in 0..<1024 {
456+
nonConsumingUse(k) // expected-note {{use here}}
457+
}
458+
self = KlassWrapper(k: Klass())
459+
}
460+
print("foo bar")
461+
}
394462
}
395463

396464
////////////////

0 commit comments

Comments
 (0)