ManualOwnership: fix simple var assign

Was mistakenly counting a 'store' as
a copying instruction, when it's only a
consuming one.

SILGen was not handling lvalues that are
addresses for loadable types correctly,
when emitting a CopyExpr in ManualOwnership.

Author: kavon

lib/SILGen/SILGenExpr.cpp

@@ -7283,10 +7283,17 @@ RValue RValueEmitter::visitCopyExpr(CopyExpr *E, SGFContext C) {
     // If we're relying on ManualOwnership for explicit-copies enforcement,
     // avoid doing address-based emission for loadable types.
     if (subType.isLoadableOrOpaque(SGF.F) && SGF.B.hasManualOwnershipAttr()) {
-      // Interpret this 'load' as a borrow + copy instead.
+      // Do a read on the lvalue. If we get back an address, do a load before
+      // emitting the explicit copy.
       LValue lv =
         SGF.emitLValue(li->getSubExpr(), SGFAccessKind::BorrowedObjectRead);
-      auto value = SGF.emitBorrowedLValue(li, std::move(lv));
+      auto value = SGF.emitRawProjectedLValue(E, std::move(lv));
+      if (value.getType().isAddress()) {
+        // We don't have 'load [explicit_copy]' so do this instead:
+        //   %x = load [copy] %value
+        //   %y = explicit_copy_value %x
+        value = SGF.emitManagedLoadCopy(E, value.getUnmanagedValue());
+      }
       ManagedValue copy = SGF.B.createExplicitCopyValue(E, value);
       return RValue(SGF, {copy}, subType.getASTType());
     }

lib/SILOptimizer/Mandatory/PerformanceDiagnostics.cpp

@@ -383,6 +383,7 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
       case SILInstructionKind::PartialApplyInst:
       case SILInstructionKind::DestroyAddrInst:
       case SILInstructionKind::DestroyValueInst:
+      case SILInstructionKind::StoreInst:
         break; // These modify reference counts, but aren't copies.
       case SILInstructionKind::ExplicitCopyAddrInst:
       case SILInstructionKind::ExplicitCopyValueInst:
@@ -441,7 +442,11 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
 
             // There's no hope of borrowing access if there's a consuming use.
             for (auto op : svi->getUses()) {
-              if (op->getOperandOwnership() == OperandOwnership::ForwardingConsume) {
+              auto useKind = op->getOperandOwnership();
+
+              // Only some DestroyingConsume's, like 'store', are interesting.
+              if (useKind == OperandOwnership::ForwardingConsume
+                  || isa<StoreInst>(op->getUser())) {
                 LLVM_DEBUG(llvm::dbgs() << "demanded by "<< *(op->getUser()));
                 diagnose(loc, diag::manualownership_copy_demanded, *name);
                 return false;

test/SIL/manual_ownership.swift

@@ -74,7 +74,7 @@ func return_borrowed_fixed(_ t: borrowing Triangle) -> Triangle {
 
 // FIXME: copy propagation isn't able to simplify this. No copy should be required.
 @_manualOwnership
-func return_consumingParam(_ t: consuming Triangle) -> Triangle { // expected-error {{accessing 't' produces a copy of it}}
+func return_consumingParam(_ t: consuming Triangle) -> Triangle { // expected-error {{ownership of 't' is demanded and cannot not be consumed}}
   return t
 }
 
@@ -156,13 +156,32 @@ func basic_function_call(_ t1: Triangle) {
 
 /// MARK: control-flow
 
+@_manualOwnership
+func check_vars(_ t: Triangle, _ b: Bool) -> Triangle {
+  var x = Triangle()
+  if b { x = t } // expected-error {{ownership of 't' is demanded and cannot not be consumed}}
+  return x // expected-error {{ownership of 'x' is demanded and cannot not be consumed}}
+}
+@_manualOwnership
+func check_vars_fixed(_ t: Triangle, _ b: Bool) -> Triangle {
+  var x = Triangle()
+  if b { x = copy t }
+  return copy x
+}
 
-// FIXME: var assignments are somtimes impossible to satisfy with 'copy'
+// FIXME: var's still have some issues.
+// (1) MandatoryRedundantLoadElimination introduces a 'copy_value' in place of a 'load [copy]'
 
+// @_manualOwnership
+// func reassignments_0() -> Triangle {
+//   var t3 = Triangle()
+//   t3 = Triangle()
+//   return t3
+// }
 // @_manualOwnership
 // func reassignments_1() {
 //   var t3 = Triangle()
-//   t3 = copy Triangle()  // FIXME: should not be needed
+//   t3 = Triangle()
 //   t3.borrowing()
 // }
 // @_manualOwnership

test/SILGen/manual_ownership.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend %s -emit-silgen -verify \
+// RUN: %target-swift-frontend %s -emit-silgen -verify -sil-verify-all \
 // RUN:   -enable-experimental-feature ManualOwnership -o %t.silgen
 
 // RUN: %FileCheck %s --input-file %t.silgen
@@ -15,9 +15,7 @@ class TriangleClass {
 // CHECK-NEXT:    debug_value %0
 // CHECK-NEXT:    [[M:%.*]] = class_method %0, #TriangleClass.shape!getter
 // CHECK-NEXT:    [[SHAPE:%.*]] = apply [[M]](%0)
-// CHECK-NEXT:    [[B:%.*]] = begin_borrow [[SHAPE]]
-// CHECK-NEXT:    [[COPY:%.*]] = explicit_copy_value [[B]]
-// CHECK-NEXT:    end_borrow [[B]]
+// CHECK-NEXT:    [[COPY:%.*]] = explicit_copy_value [[SHAPE]]
 // CHECK-NEXT:    destroy_value [[SHAPE]]
 // CHECK-NEXT:    return [[COPY]]
 // CHECK-NEXT:  } // end sil function 'basic_access_of_loadable'
@@ -27,6 +25,30 @@ func basic_access_of_loadable(_ t: TriangleClass) -> ShapeClass {
   return copy t.shape
 }
 
+// CHECK-LABEL: sil {{.*}} @var_assign
+// CHECK:          alloc_box ${ var TriangleClass }, var, name "x"
+// CHECK-NEXT:     begin_borrow [lexical] [var_decl]
+// CHECK-NEXT:     [[VAR:%.*]] = project_box {{.*}}, 0
+// CHECK:         bb1:
+// CHECK-NEXT:      [[T_COPY:%.*]] = explicit_copy_value %0
+// CHECK-NEXT:      [[ADDR1:%.*]] = begin_access [modify] [unknown] [[VAR]]
+// CHECK-NEXT:      assign [[T_COPY]] to [[ADDR1]]
+// CHECK-NEXT:      end_access [[ADDR1]]
+// CHECK:         bb3:
+// CHECK-NEXT:      [[ADDR2:%.*]] = begin_access [read] [unknown] [[VAR]]
+// CHECK-NEXT:      [[LOAD:%.*]] = load [copy] [[ADDR2]]
+// CHECK-NEXT:      [[X_COPY:%.*]] = explicit_copy_value [[LOAD]]
+// CHECK-NEXT:      end_access [[ADDR2]]
+// CHECK:           return [[X_COPY]]
+// CHECK-NEXT:    } // end sil function 'var_assign'
+@_manualOwnership
+@_silgen_name("var_assign")
+func var_assign(_ t: TriangleClass, _ b: Bool) -> TriangleClass {
+  var x = TriangleClass()
+  if b { x = copy t }
+  return copy x
+}
+
 // CHECK-LABEL: sil {{.*}} [manual_ownership] [ossa] @return_borrowed
 // CHECK:       bb0(%0 : @guaranteed $TriangleClass):
 // CHECK-NEXT:     debug_value %0