[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.

Author: gottesmm

lib/SILOptimizer/Mandatory/MoveFunctionCanonicalization.cpp

@@ -113,7 +113,10 @@ tryHandlingLoadableVarMovePattern(MarkUnresolvedMoveAddrInst *markMoveAddr,
   LLVM_DEBUG(llvm::dbgs() << "Found LI: " << *li);
   SILValue operand = stripAccessMarkers(li->getOperand());
   auto *originalASI = dyn_cast<AllocStackInst>(operand);
-  if (!originalASI || !originalASI->isLexical() || !originalASI->isVar())
+  // 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;
 
   LLVM_DEBUG(llvm::dbgs() << "Found OriginalASI: " << *originalASI);

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

@@ -162,7 +162,7 @@ static llvm::cl::opt<bool>
     DisableUnhandledMoveDiagnostic("sil-disable-unknown-moveaddr-diagnostic");
 
 //===----------------------------------------------------------------------===//
-//                            Diagnostic Utilities
+//                                 Utilities
 //===----------------------------------------------------------------------===//
 
 template <typename... T, typename... U>
@@ -171,6 +171,15 @@ static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag,
   Context.Diags.diagnose(loc, diag, std::forward<U>(args)...);
 }
 
+namespace llvm {
+llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const SmallBitVector &bv) {
+  for (unsigned index : range(bv.size())) {
+    os << (bv[index] ? 1 : 0);
+  }
+  return os;
+}
+} // namespace llvm
+
 //===----------------------------------------------------------------------===//
 //                               Use Gathering
 //===----------------------------------------------------------------------===//
@@ -610,29 +619,36 @@ cleanupAllDestroyAddr(SILFunction *fn, SmallBitVector &destroyIndices,
   bool madeChange = false;
   BasicBlockWorklist worklist(fn);
   SILValue daiOperand;
+  LLVM_DEBUG(llvm::dbgs() << "Cleanup up destroy addr!\n");
+  LLVM_DEBUG(llvm::dbgs() << "    Visiting destroys!\n");
+  LLVM_DEBUG(llvm::dbgs() << "    Destroy Indices: " << destroyIndices << "\n");
   for (int index = destroyIndices.find_first(); index != -1;
        index = destroyIndices.find_next(index)) {
+    LLVM_DEBUG(llvm::dbgs() << "    Index: " << index << "\n");
     auto dai = useState.destroys[index];
     if (!dai)
       continue;
+    LLVM_DEBUG(llvm::dbgs() << "    Destroy: " << *dai);
     SILValue op = (*dai)->getOperand();
     assert(daiOperand == SILValue() || op == daiOperand);
     daiOperand = op;
     for (auto *predBlock : (*dai)->getParent()->getPredecessorBlocks()) {
       worklist.pushIfNotVisited(predBlock);
     }
   }
-
+  LLVM_DEBUG(llvm::dbgs() << "    Visiting reinit!\n");
   for (int index = reinitIndices.find_first(); index != -1;
        index = reinitIndices.find_next(index)) {
     auto reinit = useState.reinits[index];
     if (!reinit)
       continue;
+    LLVM_DEBUG(llvm::dbgs() << "  Reinit: " << *reinit);
     for (auto *predBlock : (*reinit)->getParent()->getPredecessorBlocks()) {
       worklist.pushIfNotVisited(predBlock);
     }
   }
 
+  LLVM_DEBUG(llvm::dbgs() << "Processing worklist!\n");
   while (auto *next = worklist.pop()) {
     LLVM_DEBUG(llvm::dbgs()
                << "Looking at block: bb" << next->getDebugID() << "\n");
@@ -699,10 +715,6 @@ bool MoveKillsCopyableAddressesObjectChecker::performGlobalDataflow(
   BasicBlockSet blocksWithMovesThatAreNowTakes(fn);
   bool convertedMarkMoveToTake = false;
   for (auto *mvi : markMovesToDataflow) {
-    // At the end of this if we already initialized indices of paired destroys,
-    // clear it.
-    SWIFT_DEFER { getIndicesOfPairedDestroys().clear(); };
-
     bool emittedSingleDiagnostic = false;
 
     LLVM_DEBUG(llvm::dbgs() << "Checking Multi Block Dataflow for: " << *mvi);

test/SILOptimizer/move_function_kills_copyable_loadable_vars.swift

@@ -93,3 +93,23 @@ public func performMoveOnVarMultiBlockError2(_ p: Klass) {
     x = p
     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) {
+    let x: Klass // expected-error {{'x' used after being moved}}
+    do {
+        x = p
+    }
+    let _ = _move(x) // expected-note {{move here}}
+    nonConsumingUse(x) // expected-note {{use here}}
+}
+
+public func performMoveOnLaterDefinedInit2(_ p: Klass) {
+    let x: Klass
+    do {
+        x = p
+    }
+    nonConsumingUse(x)
+    let _ = _move(x)
+}

test/lit.cfg

@@ -901,6 +901,7 @@ def use_interpreter_for_simple_runs():
     config.target_run_stdlib_swift = make_simple_target_run(stdlib=True)
     config.target_run_simple_swift = make_simple_target_run()
     config.target_run_simple_swift_parameterized = make_simple_target_run(parameterized=True)
+    config.target_run_stdlib_swift_parameterized = make_simple_target_run(stdlib=True, parameterized=True)
     config.target_run_simple_swiftgyb_parameterized = make_simple_target_run(gyb=True, parameterized=True)
     config.available_features.add('interpret')
 
@@ -2097,6 +2098,17 @@ if not getattr(config, 'target_run_simple_swift', None):
           escape_for_substitute_captures(config.target_codesign),
           escape_for_substitute_captures(config.target_run))
     )
+    config.target_run_stdlib_swift_parameterized = SubstituteCaptures(
+        r"%%empty-directory(%%t) && "
+        r"%s %s %%s \1 -o %%t/a.out -module-name main "
+        r"    -Xfrontend -disable-access-control && "
+        r"%s %%t/a.out && "
+        r"%s %%t/a.out"
+        % (escape_for_substitute_captures(config.target_build_swift),
+           escape_for_substitute_captures(mcp_opt),
+           escape_for_substitute_captures(config.target_codesign),
+           escape_for_substitute_captures(config.target_run))
+    )
 
     config.target_run_simple_swift = (
         '%%empty-directory(%%t) && '
@@ -2187,6 +2199,9 @@ config.substitutions.append(('%target-run-simple-swift\(([^)]+)\)',
                             config.target_run_simple_swift_parameterized))
 config.substitutions.append(('%target-fail-simple-swift\(([^)]+)\)',
                             config.target_fail_simple_swift_parameterized))
+config.substitutions.append(('%target-run-stdlib-swift\(([^)]+)\)',
+                            config.target_run_stdlib_swift_parameterized))
+
 config.substitutions.append(('%target-run-simple-swift', config.target_run_simple_swift))
 config.substitutions.append(('%target-run-stdlib-swiftgyb', config.target_run_stdlib_swiftgyb))
 config.substitutions.append(('%target-run-stdlib-swift', config.target_run_stdlib_swift))

test/stdlib/move_function.swift

@@ -0,0 +1,90 @@
+// RUN: %target-run-stdlib-swift(-Xllvm -sil-disable-pass=DestroyHoisting)
+//
+// NOTE ON ABOVE: I am disabling destroy hoisting on this test since we are
+// going to move it out of the mandatory pipeline eventually and it causes the
+// test to fail only when optimizations are enabled.
+
+// REQUIRES: executable_test
+
+import Swift
+import StdlibUnittest
+
+public class Klass {}
+
+var tests = TestSuite("move_function_uniqueness")
+
+public enum Enum {
+    case foo
+}
+
+public class K2 {
+    var array: [Enum] = Array(repeating: .foo, count: 10_000)
+
+    subscript(index: Int) -> Enum {
+        @inline(never)
+        get {
+            array[index]
+        }
+        @inline(never)
+        set {
+            array[index] = newValue
+        }
+        @inline(never)
+        _modify {
+            yield &array[index]
+        }
+    }
+}
+
+public class Class {
+    var k2 = K2()
+    var array: [Enum] = Array(repeating: .foo, count: 10_000)
+}
+
+extension Class {
+    @inline(never)
+    func readClassTest(_ userHandle: Int) {
+        assert(_isUnique(&self.k2))
+
+        let x: K2
+        do {
+            x = self.k2
+        }
+        switch _move(x)[userHandle] {
+        case .foo:
+            assert(_isUnique(&self.k2))
+        }
+    }
+}
+
+tests.test("classUniquenessTest") {
+    let c = Class()
+    for f in 0..<10_000 {
+        c.readClassTest(f)
+    }
+}
+
+extension Class {
+    @inline(never)
+    func readArrayTest(_ userHandle: Int) {
+        assert(self.array._buffer.isUniquelyReferenced())
+
+        let x: [Enum]
+        do {
+            x = self.array
+        }
+        switch _move(x)[userHandle] {
+        case .foo:
+            assert(self.array._buffer.isUniquelyReferenced())
+        }
+    }
+}
+
+tests.test("arrayUniquenessTest") {
+    let c = Class()
+    for f in 0..<10_000 {
+        c.readArrayTest(f)
+    }
+}
+
+runAllTests()