[move-function] A few additions

lib/SILOptimizer/Mandatory/MoveFunctionCanonicalization.cpp

@@ -112,14 +112,18 @@ tryHandlingLoadableVarMovePattern(MarkUnresolvedMoveAddrInst *markMoveAddr,
 
   LLVM_DEBUG(llvm::dbgs() << "Found LI: " << *li);
   SILValue operand = stripAccessMarkers(li->getOperand());
-  auto *originalASI = dyn_cast<AllocStackInst>(operand);
-  // Make sure that we have a lexical alloc_stack marked as a var or a let. We
-  // don't want properties.
-  if (!originalASI || !originalASI->isLexical() ||
-      !(originalASI->isVar() || originalASI->isLet()))
-    return false;
+  if (auto *originalASI = dyn_cast<AllocStackInst>(operand)) {
+    if (!originalASI->isLexical() ||
+        !(originalASI->isVar() || originalASI->isLet()))
+      return false;
+    LLVM_DEBUG(llvm::dbgs() << "Found OriginalASI: " << *originalASI);
+  } else {
+    auto *fArg = dyn_cast<SILFunctionArgument>(operand);
+    if (!fArg || !fArg->hasConvention(SILArgumentConvention::Indirect_Inout))
+      return false;
+    LLVM_DEBUG(llvm::dbgs() << "Found fArg: " << *fArg);
+  }
 
-  LLVM_DEBUG(llvm::dbgs() << "Found OriginalASI: " << *originalASI);
   // Make sure that there aren't any side-effect having instructions in
   // between our load/store.
   LLVM_DEBUG(llvm::dbgs() << "Checking for uses in between LI and SI.\n");
@@ -132,7 +136,7 @@ tryHandlingLoadableVarMovePattern(MarkUnresolvedMoveAddrInst *markMoveAddr,
         }
 
         if (auto *dvi = dyn_cast<DestroyAddrInst>(&iter)) {
-          if (aa->isNoAlias(dvi->getOperand(), originalASI)) {
+          if (aa->isNoAlias(dvi->getOperand(), operand)) {
             // We are going to be extending the lifetime of our
             // underlying value, not shrinking it so we can ignore
             // destroy_addr on other non-aliasing values.
@@ -157,7 +161,7 @@ tryHandlingLoadableVarMovePattern(MarkUnresolvedMoveAddrInst *markMoveAddr,
   // Ok, we know our original lexical alloc_stack is not written to in between
   // the load/store. Move the mark_move_addr onto the lexical alloc_stack.
   LLVM_DEBUG(llvm::dbgs() << "        Doing loadable var!\n");
-  markMoveAddr->setSrc(originalASI);
+  markMoveAddr->setSrc(operand);
   return true;
 }
 
@@ -239,13 +243,13 @@ static bool tryConvertSimpleMoveFromAllocStackTemporary(
   }
 
   // If we have a store [init], see if our src is a load [copy] from an
-  // alloc_stack that is lexical var. In this case, we want to move our
-  // mark_unresolved_move_addr onto that lexical var. This pattern occurs due to
-  // SILGen always loading loadable values from memory when retrieving an
-  // RValue. Calling _move then since _move is generic forces the value to be
-  // re-materialized into an alloc_stack. In this example remembering that
-  // mark_unresolved_move_addr is a copy_addr [init], we try to move the MUMA
-  // onto the original lexical alloc_stack.
+  // alloc_stack that is lexical var or an inout argument. In this case, we want
+  // to move our mark_unresolved_move_addr onto that lexical var. This pattern
+  // occurs due to SILGen always loading loadable values from memory when
+  // retrieving an RValue. Calling _move then since _move is generic forces the
+  // value to be re-materialized into an alloc_stack. In this example
+  // remembering that mark_unresolved_move_addr is a copy_addr [init], we try to
+  // move the MUMA onto the original lexical alloc_stack.
   if (tryHandlingLoadableVarMovePattern(markMoveAddr, si, aa))
     return true;
 

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

@@ -885,6 +885,20 @@ bool MoveKillsCopyableAddressesObjectChecker::check() {
     if (!visitAccessPathUses(visitor, accessPath, fn))
       continue;
 
+    // See if our base address is an inout. If we found any moves, add as a
+    // liveness use all function terminators.
+    if (auto *fArg = dyn_cast<SILFunctionArgument>(address)) {
+      if (fArg->hasConvention(SILArgumentConvention::Indirect_Inout)) {
+        if (visitor.useState.markMoves.size()) {
+          SmallVector<SILBasicBlock *, 2> exitingBlocks;
+          fn->findExitingBlocks(exitingBlocks);
+          for (auto *block : exitingBlocks) {
+            visitor.useState.livenessUses.insert(block->getTerminator());
+          }
+        }
+      }
+    }
+
     // Now initialize our data structures.
     SWIFT_DEFER {
       useBlocks.clear();
@@ -987,7 +1001,7 @@ class MoveKillsCopyableAddressesCheckerPass : public SILFunctionTransform {
     for (auto *arg : fn->front().getSILFunctionArguments()) {
       if (arg->getType().isAddress() &&
           (arg->hasConvention(SILArgumentConvention::Indirect_In) ||
-           arg->hasConvention(SILArgumentConvention::Indirect_In)))
+           arg->hasConvention(SILArgumentConvention::Indirect_Inout)))
         checker.addressesToCheck.insert(arg);
     }
 

lib/SILOptimizer/Mandatory/MoveKillsCopyableValuesChecker.cpp

@@ -335,8 +335,7 @@ bool MoveKillsCopyableValuesChecker::check() {
   SmallSetVector<SILValue, 32> valuesToCheck;
 
   for (auto *arg : fn->getEntryBlock()->getSILFunctionArguments()) {
-    if (arg->getOwnershipKind() == OwnershipKind::Guaranteed ||
-        arg->getOwnershipKind() == OwnershipKind::Owned)
+    if (arg->getOwnershipKind() == OwnershipKind::Owned)
       valuesToCheck.insert(arg);
   }
 

test/SILOptimizer/move_function_kills_copyable_addressonly_vars.swift

@@ -13,6 +13,7 @@ public class Klass {}
 func consumingUse<T>(_ k: __owned T) {}
 var booleanValue: Bool { false }
 func nonConsumingUse<T>(_ k: T) {}
+func exchangeUse<T>(_ k: __owned T) -> T { k }
 
 ///////////
 // Tests //
@@ -93,3 +94,106 @@ public func performMoveOnVarMultiBlockError2<T>(_ p: T) {
     x = p
     nonConsumingUse(x)
 }
+
+public func performMoveOnInOut<T>(_ p: inout T) { // expected-error {{'p' used after being moved}}
+    let buf = _move(p) // expected-note {{move here}}
+    let _ = buf
+} // expected-note {{use here}}
+
+public func performMoveOnInOut2<T>(_ p: inout T, _ p2: T) {
+    let buf = _move(p)
+    let _ = buf
+    p = p2
+}
+
+struct S<T> {
+    var buffer: T?
+
+    mutating func appendNoError() {
+        let b = _move(self).buffer
+        let maybeNewB = exchangeUse(b)
+        self = .init(buffer: maybeNewB)
+    }
+
+    mutating func appendError() { // expected-error {{'self' used after being moved}}
+        let b = _move(self).buffer // expected-note {{move here}}
+        let _ = b
+    } // expected-note {{use here}}
+
+    mutating func appendThrowingNoError1(_ f: () throws -> ()) throws {
+        let b = _move(self).buffer
+        let maybeNewB = exchangeUse(b)
+        // We have to initialize self before we call try since otherwise we will
+        // not initialize self along the throws path.
+        self = .init(buffer: maybeNewB)
+        try f()
+    }
+
+    mutating func appendThrowingNoError2(_ f: () throws -> ()) {
+        do {
+            let b = _move(self).buffer
+            try f()
+            let maybeNewB = exchangeUse(b)
+            self = .init(buffer: maybeNewB)
+        } catch {
+            self = .init(buffer: nil)
+        }
+    }
+
+    // In this case, since we initialize self before the try point, we will have
+    // re-initialized self before hitting either the code after the try that is
+    // inline or the catch block.
+    mutating func appendThrowingNoError3(_ f: () throws -> ()) {
+        do {
+            let b = _move(self).buffer
+            let maybeNewB = exchangeUse(b)
+            self = .init(buffer: maybeNewB)
+            try f()
+        } catch {
+        }
+    }
+
+    mutating func appendThrowingError0(_ f: () throws -> ()) throws { // expected-error {{'self' used after being moved}}
+        let b = _move(self).buffer // expected-note {{move here}}
+        let maybeNewB = exchangeUse(b)
+        try f() // expected-note {{use here}}
+        self = .init(buffer: maybeNewB)
+    }
+
+
+    mutating func appendThrowingError1(_ f: () throws -> ()) throws { // expected-error {{'self' used after being moved}}
+        let b = _move(self).buffer // expected-note {{move here}}
+        let maybeNewB = exchangeUse(b)
+        let _ = maybeNewB
+        try f() // expected-note {{use here}}
+    }
+
+    mutating func appendThrowingError2(_ f: () throws -> ()) { // expected-error {{'self' used after being moved}}
+        do {
+            let b = _move(self).buffer // expected-note {{move here}}
+            let _ = b
+            try f()
+        } catch {
+            self = .init(buffer: nil)
+        }
+    } // expected-note {{use here}}
+
+    mutating func appendThrowingError3(_ f: () throws -> ()) { // expected-error {{'self' used after being moved}}
+        do {
+            let b = _move(self).buffer // expected-note {{move here}}
+            try f()
+            let maybeNewB = exchangeUse(b)
+            self = .init(buffer: maybeNewB)
+        } catch {
+        }
+    } // expected-note {{use here}}
+
+    mutating func appendThrowingError4(_ f: () throws -> ()) { // expected-error {{'self' used after being moved}}
+        do {
+            let b = _move(self).buffer // expected-note {{move here}}
+            let _ = b
+            try f()
+        } catch {
+        }
+    } // expected-note {{use here}}
+}

test/SILOptimizer/move_function_kills_copyable_loadable_vars.swift

@@ -12,7 +12,9 @@ public class Klass {}
 
 func consumingUse(_ k: __owned Klass) {}
 var booleanValue: Bool { false }
+var booleanValue2: Bool { false }
 func nonConsumingUse(_ k: Klass) {}
+func exchangeUse(_ k: Klass) -> Klass { k }
 
 ///////////
 // Klassests //
@@ -94,6 +96,57 @@ public func performMoveOnVarMultiBlockError2(_ p: Klass) {
     nonConsumingUse(x)
 }
 
+public func performMoveConditionalReinitalization(_ p: Klass) {
+    var x = p
+
+    if booleanValue {
+        nonConsumingUse(x)
+        let _ = _move(x)
+        x = p
+        nonConsumingUse(x)
+    } else {
+        nonConsumingUse(x)
+    }
+
+    nonConsumingUse(x)
+}
+
+public func performMoveConditionalReinitalization2(_ p: Klass) {
+    var x = p // expected-error {{'x' used after being moved}}
+
+    if booleanValue {
+        nonConsumingUse(x)
+        let _ = _move(x) // expected-note {{move here}}
+        nonConsumingUse(x) // expected-note {{use here}}
+        x = p
+        nonConsumingUse(x)
+    } else {
+        nonConsumingUse(x)
+    }
+
+    nonConsumingUse(x)
+}
+
+public func performMoveConditionalReinitalization3(_ p: Klass, _ p2: Klass, _ p3: Klass) {
+    var x = p // expected-error {{'x' used after being moved}}
+              // expected-error @-1 {{'x' used after being moved}}
+
+    if booleanValue {
+        nonConsumingUse(x)
+        let _ = _move(x)   // expected-note {{move here}}
+        nonConsumingUse(x) // expected-note {{use here}}
+        nonConsumingUse(x) // We only emit for the first one.
+        x = p2
+        nonConsumingUse(x)
+        let _ = _move(x)   // expected-note {{move here}}
+        nonConsumingUse(x) // expected-note {{use here}}
+    } else {
+        nonConsumingUse(x)
+    }
+
+    nonConsumingUse(x)
+}
+
 // Even though the examples below are for lets, since the let is not initially
 // defined it comes out like a var.
 public func performMoveOnLaterDefinedInit(_ p: Klass) {
@@ -113,3 +166,107 @@ public func performMoveOnLaterDefinedInit2(_ p: Klass) {
     nonConsumingUse(x)
     let _ = _move(x)
 }
+
+public func performMoveOnInOut(_ p: inout Klass) { // expected-error {{'p' used after being moved}}
+    let buf = _move(p) // expected-note {{move here}}
+    let _ = buf
+} // expected-note {{use here}}
+
+public func performMoveOnInOut2(_ p: inout Klass, _ p2: Klass) {
+    let buf = _move(p)
+    p = p2
+    let _ = buf
+}
+
+// Mutating self is an inout argument.
+struct S {
+    var buffer: Klass?
+
+    mutating func appendNoError() {
+        let b = _move(self).buffer!
+        let maybeNewB = exchangeUse(b)
+        self = .init(buffer: maybeNewB)
+    }
+
+    mutating func appendError() { // expected-error {{'self' used after being moved}}
+        let b = _move(self).buffer // expected-note {{move here}}
+        let _ = b
+    } // expected-note {{use here}}
+
+    mutating func appendThrowingNoError1(_ f: () throws -> ()) throws {
+        let b = _move(self).buffer!
+        let maybeNewB = exchangeUse(b)
+        // We have to initialize self before we call try since otherwise we will
+        // not initialize self along the throws path.
+        self = .init(buffer: maybeNewB)
+        try f()
+    }
+
+    mutating func appendThrowingNoError2(_ f: () throws -> ()) {
+        do {
+            let b = _move(self).buffer!
+            try f()
+            let maybeNewB = exchangeUse(b)
+            self = .init(buffer: maybeNewB)
+        } catch {
+            self = .init(buffer: nil)
+        }
+    }
+
+    // In this case, since we initialize self before the try point, we will have
+    // re-initialized self before hitting either the code after the try that is
+    // inline or the catch block.
+    mutating func appendThrowingNoError3(_ f: () throws -> ()) {
+        do {
+            let b = _move(self).buffer!
+            let maybeNewB = exchangeUse(b)
+            self = .init(buffer: maybeNewB)
+            try f()
+        } catch {
+        }
+    }
+
+    mutating func appendThrowingError0(_ f: () throws -> ()) throws { // expected-error {{'self' used after being moved}}
+        let b = _move(self).buffer! // expected-note {{move here}}
+        let maybeNewB = exchangeUse(b)
+        try f() // expected-note {{use here}}
+        self = .init(buffer: maybeNewB)
+    }
+
+
+    mutating func appendThrowingError1(_ f: () throws -> ()) throws { // expected-error {{'self' used after being moved}}
+        let b = _move(self).buffer! // expected-note {{move here}}
+        let maybeNewB = exchangeUse(b)
+        let _ = maybeNewB
+        try f() // expected-note {{use here}}
+    }
+
+    mutating func appendThrowingError2(_ f: () throws -> ()) { // expected-error {{'self' used after being moved}}
+        do {
+            let b = _move(self).buffer // expected-note {{move here}}
+            let _ = b
+            try f()
+        } catch {
+            self = .init(buffer: nil)
+        }
+    } // expected-note {{use here}}
+
+    mutating func appendThrowingError3(_ f: () throws -> ()) { // expected-error {{'self' used after being moved}}
+        do {
+            let b = _move(self).buffer! // expected-note {{move here}}
+            try f()
+            let maybeNewB = exchangeUse(b)
+            self = .init(buffer: maybeNewB)
+        } catch {
+        }
+    } // expected-note {{use here}}
+
+    mutating func appendThrowingError4(_ f: () throws -> ()) { // expected-error {{'self' used after being moved}}
+        do {
+            let b = _move(self).buffer // expected-note {{move here}}
+            let _ = b
+            try f()
+        } catch {
+        }
+    } // expected-note {{use here}}
+}