Skip to content

Commit ee8fd04

Browse files
committed
[move-keyword] Make sure we error correctly on f(_move y, &y) and f(&y, _move y)
This was actually crashing on the inout use since I at some point changed the isReinit code to use a more general routine to determine that which was incorrect. Instead, this is actually trying to recognize reinits that we know how to change into a non-reinit form (e.x.: store [assign] -> store [init]). Since the inout use was not handled by the convert reinit to init code, we crashed. Now we get the correct behavior since the access to y always occurs /before/ the formal evaluation of the inout.
1 parent b0256b9 commit ee8fd04

File tree

4 files changed

+91
-2
lines changed

4 files changed

+91
-2
lines changed

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,24 @@ struct ClosureOperandState {
264264

265265
} // namespace
266266

267+
/// Is this a reinit instruction that we know how to convert into its init form.
268+
static bool isReinitToInitConvertibleInst(Operand *memUse) {
269+
auto *memInst = memUse->getUser();
270+
switch (memInst->getKind()) {
271+
default:
272+
return false;
273+
274+
case SILInstructionKind::CopyAddrInst: {
275+
auto *cai = cast<CopyAddrInst>(memInst);
276+
return !cai->isInitializationOfDest();
277+
}
278+
case SILInstructionKind::StoreInst: {
279+
auto *si = cast<StoreInst>(memInst);
280+
return si->getOwnershipQualifier() == StoreOwnershipQualifier::Assign;
281+
}
282+
}
283+
}
284+
267285
static void convertMemoryReinitToInitForm(SILInstruction *memInst) {
268286
switch (memInst->getKind()) {
269287
default:
@@ -932,7 +950,7 @@ bool GatherClosureUseVisitor::visitUse(Operand *op, AccessUseType useTy) {
932950
return true;
933951
}
934952

935-
if (memInstMustReinitialize(op)) {
953+
if (isReinitToInitConvertibleInst(op)) {
936954
if (stripAccessMarkers(op->get()) != useState.address) {
937955
LLVM_DEBUG(llvm::dbgs()
938956
<< "!!! Error! Found reinit use not on base address: "
@@ -1308,7 +1326,7 @@ bool GatherLexicalLifetimeUseVisitor::visitUse(Operand *op,
13081326
return true;
13091327
}
13101328

1311-
if (memInstMustReinitialize(op)) {
1329+
if (isReinitToInitConvertibleInst(op)) {
13121330
if (stripAccessMarkers(op->get()) != useState.address) {
13131331
LLVM_DEBUG(llvm::dbgs()
13141332
<< "!!! Error! Found reinit use not on base address: "
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all -o - -sil-move-kills-copyable-addresses-checker -verify %s
2+
3+
// This file is meant for specific SIL patterns that may be hard to produce with
4+
// the current swift frontend but have reproduced before and we want to make
5+
// sure keep on working.
6+
7+
sil_stage raw
8+
9+
class Klass {}
10+
11+
sil @useTwice : $@convention(thin) (@guaranteed Klass, @inout Klass) -> ()
12+
13+
sil hidden [ossa] @$s4test3fooyyAA5KlassCnF : $@convention(thin) (@owned Klass) -> () {
14+
bb0(%0 : @owned $Klass):
15+
%1 = begin_borrow [lexical] %0 : $Klass
16+
debug_value %1 : $Klass, let, name "k", argno 1
17+
%3 = alloc_stack [lexical] $Klass, var, name "k2" // expected-error {{'k2' used after being moved}}
18+
%4 = copy_value %1 : $Klass
19+
%5 = move_value [allows_diagnostics] %4 : $Klass
20+
store %5 to [init] %3 : $*Klass
21+
%7 = begin_access [modify] [static] %3 : $*Klass
22+
%8 = alloc_stack $Klass
23+
mark_unresolved_move_addr %7 to %8 : $*Klass // expected-note {{move here}}
24+
%10 = load [copy] %8 : $*Klass
25+
destroy_addr %8 : $*Klass
26+
end_access %7 : $*Klass
27+
%13 = begin_access [modify] [static] %3 : $*Klass
28+
%14 = function_ref @useTwice : $@convention(thin) (@guaranteed Klass, @inout Klass) -> ()
29+
%15 = apply %14(%10, %13) : $@convention(thin) (@guaranteed Klass, @inout Klass) -> () // expected-note {{use here}}
30+
end_access %13 : $*Klass
31+
destroy_value %10 : $Klass
32+
dealloc_stack %8 : $*Klass
33+
destroy_addr %3 : $*Klass
34+
dealloc_stack %3 : $*Klass
35+
end_borrow %1 : $Klass
36+
destroy_value %0 : $Klass
37+
%23 = tuple ()
38+
return %23 : $()
39+
}

test/SILOptimizer/move_function_kills_copyable_addressonly_vars.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,3 +688,19 @@ func reinitInPieces1<T : P>(_ k: ProtPair<T>) {
688688
k2.lhs = selfType.getP()
689689
k2.rhs = selfType.getP()
690690
}
691+
692+
////////////////////////
693+
// InOut and Use Test //
694+
////////////////////////
695+
696+
func useValueAndInOut<T>(_ x: T, _ y: inout T) {}
697+
func useValueAndInOut<T>(_ x: inout T, _ y: T) {}
698+
699+
func inoutAndUseTest<T>(_ x: T) {
700+
var y = x // expected-error {{'y' used after being moved}}
701+
// expected-error @-1 {{'y' used after being moved}}
702+
useValueAndInOut(_move y, &y) // expected-note {{use here}}
703+
// expected-note @-1 {{move here}}
704+
useValueAndInOut(&y, _move y) // expected-note {{use here}}
705+
// expected-note @-1 {{move here}}
706+
}

test/SILOptimizer/move_function_kills_copyable_loadable_vars.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,3 +729,19 @@ func reinitInPieces1(_ k: KlassPair) {
729729
k2.lhs = Klass()
730730
k2.rhs = Klass()
731731
}
732+
733+
////////////////////////
734+
// InOut and Use Test //
735+
////////////////////////
736+
737+
func useValueAndInOut(_ x: Klass, _ y: inout Klass) {}
738+
func useValueAndInOut(_ x: inout Klass, _ y: Klass) {}
739+
740+
func inoutAndUseTest(_ x: Klass) {
741+
var y = x // expected-error {{'y' used after being moved}}
742+
// expected-error @-1 {{'y' used after being moved}}
743+
useValueAndInOut(_move y, &y) // expected-note {{use here}}
744+
// expected-note @-1 {{move here}}
745+
useValueAndInOut(&y, _move y) // expected-note {{use here}}
746+
// expected-note @-1 {{move here}}
747+
}

0 commit comments

Comments
 (0)