Merge pull request #741 from swiftwasm/master

[pull] swiftwasm from master

Author: MaxDesiatov

Diff for: lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -781,10 +781,13 @@ SILInstruction *SILCombiner::visitReleaseValueInst(ReleaseValueInst *RVI) {
     }
   }
 
-  // ReleaseValueInst of an unowned type is an unowned_release.
-  if (OperandTy.is<UnownedStorageType>())
-    return Builder.createUnownedRelease(RVI->getLoc(), Operand,
+  // ReleaseValueInst of a loadable reference storage type needs the
+  // corresponding release instruction.
+#define ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...)            \
+  if (OperandTy.is<Name##StorageType>())                                       \
+    return Builder.create##Name##Release(RVI->getLoc(), Operand,               \
                                         RVI->getAtomicity());
+#include "swift/AST/ReferenceStorage.def"
 
   // ReleaseValueInst of a reference type is a strong_release.
   if (OperandTy.isReferenceCounted(RVI->getModule()))
@@ -821,10 +824,13 @@ SILInstruction *SILCombiner::visitRetainValueInst(RetainValueInst *RVI) {
     }
   }
 
-  // RetainValueInst of an unowned type is an unowned_retain.
-  if (OperandTy.is<UnownedStorageType>())
-    return Builder.createUnownedRetain(RVI->getLoc(), Operand,
+  // RetainValueInst of a loadable reference storage type needs the
+  // corresponding retain instruction.
+#define ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...)            \
+  if (OperandTy.is<Name##StorageType>())                                       \
+    return Builder.create##Name##Retain(RVI->getLoc(), Operand,                \
                                        RVI->getAtomicity());
+#include "swift/AST/ReferenceStorage.def"
 
   // RetainValueInst of a reference type is a strong_release.
   if (OperandTy.isReferenceCounted(RVI->getModule())) {

Diff for: lib/SILOptimizer/Transforms/SemanticARCOpts.cpp

@@ -52,21 +52,36 @@ class LiveRange {
   /// introducer and not to be forwarding.
   OwnedValueIntroducer introducer;
 
+  /// A vector that we store all of our uses into.
+  ///
+  /// Some properties of this array are:
+  ///
+  /// 1. It is only mutated in the constructor of LiveRange.
+  ///
+  /// 2. destroyingUses, ownershipForwardingUses, and unknownConsumingUses are
+  /// views into this array. We store the respective uses in the aforementioned
+  /// order. This is why it is important not to mutate consumingUses after we
+  /// construct the LiveRange since going from small -> large could invalidate
+  /// the uses.
+  SmallVector<Operand *, 6> consumingUses;
+
   /// A list of destroy_values of the live range.
-  SmallVector<Operand *, 2> destroyingUses;
+  ///
+  /// This is just a view into consuming uses.
+  ArrayRef<Operand *> destroyingUses;
 
   /// A list of forwarding instructions that forward owned ownership, but that
   /// are also able to be converted to guaranteed ownership. If we are able to
   /// eliminate this LiveRange due to it being from a guaranteed value, we must
   /// flip the ownership of all of these instructions to guaranteed from owned.
   ///
   /// Corresponds to isOwnershipForwardingInst(...).
-  SmallVector<Operand *, 2> ownershipForwardingUses;
+  ArrayRef<Operand *> ownershipForwardingUses;
 
   /// Consuming uses that we were not able to understand as a forwarding
   /// instruction or a destroy_value. These must be passed a strongly control
   /// equivalent +1 value.
-  SmallVector<Operand *, 2> unknownConsumingUses;
+  ArrayRef<Operand *> unknownConsumingUses;
 
 public:
   LiveRange(SILValue value);
@@ -87,6 +102,11 @@ class LiveRange {
   HasConsumingUse_t
   hasUnknownConsumingUse(bool assumingFixedPoint = false) const;
 
+  /// Return an array ref to /all/ consuming uses. Will include all 3 sorts of
+  /// consuming uses: destroying uses, forwarding consuming uses, and unknown
+  /// forwarding instruction.
+  ArrayRef<Operand *> getAllConsumingUses() const { return consumingUses; }
+
   ArrayRef<Operand *> getDestroyingUses() const { return destroyingUses; }
 
 private:
@@ -121,7 +141,7 @@ class LiveRange {
     return ownershipForwardingUses;
   }
 
-  void convertOwnedGeneralForwardingUsesToGuaranteed();
+  void convertOwnedGeneralForwardingUsesToGuaranteed() &&;
 
   /// A consuming operation that:
   ///
@@ -192,6 +212,10 @@ LiveRange::LiveRange(SILValue value)
       ownershipForwardingUses(), unknownConsumingUses() {
   assert(introducer.value.getOwnershipKind() == ValueOwnershipKind::Owned);
 
+  SmallVector<Operand *, 32> tmpDestroyingUses;
+  SmallVector<Operand *, 32> tmpForwardingConsumingUses;
+  SmallVector<Operand *, 32> tmpUnknownConsumingUses;
+
   // We know that our silvalue produces an @owned value. Look through all of our
   // uses and classify them as either consuming or not.
   SmallVector<Operand *, 32> worklist(introducer.value->getUses());
@@ -220,7 +244,7 @@ LiveRange::LiveRange(SILValue value)
     // Ok, we know now that we have a consuming use. See if we have a destroy
     // value, quickly up front. If we do have one, stash it and continue.
     if (isa<DestroyValueInst>(user)) {
-      destroyingUses.push_back(op);
+      tmpDestroyingUses.push_back(op);
       continue;
     }
 
@@ -244,13 +268,13 @@ LiveRange::LiveRange(SILValue value)
                         return v.getOwnershipKind() ==
                                ValueOwnershipKind::Owned;
                       })) {
-      unknownConsumingUses.push_back(op);
+      tmpUnknownConsumingUses.push_back(op);
       continue;
     }
 
     // Ok, this is a forwarding instruction whose ownership we can flip from
     // owned -> guaranteed.
-    ownershipForwardingUses.push_back(op);
+    tmpForwardingConsumingUses.push_back(op);
 
     // If we have a non-terminator, just visit its users recursively to see if
     // the the users force the live range to be alive.
@@ -284,6 +308,20 @@ LiveRange::LiveRange(SILValue value)
       }
     }
   }
+
+  // The order in which we append these to consumingUses matters since we assume
+  // their order as an invariant. This is done to ensure that we can pass off
+  // all of our uses or individual sub-arrays of our users without needing to
+  // move around memory.
+  llvm::copy(tmpDestroyingUses, std::back_inserter(consumingUses));
+  llvm::copy(tmpForwardingConsumingUses, std::back_inserter(consumingUses));
+  llvm::copy(tmpUnknownConsumingUses, std::back_inserter(consumingUses));
+
+  auto cUseArrayRef = llvm::makeArrayRef(consumingUses);
+  destroyingUses = cUseArrayRef.take_front(tmpDestroyingUses.size());
+  ownershipForwardingUses = cUseArrayRef.slice(
+      tmpDestroyingUses.size(), tmpForwardingConsumingUses.size());
+  unknownConsumingUses = cUseArrayRef.take_back(tmpUnknownConsumingUses.size());
 }
 
 void LiveRange::insertEndBorrowsAtDestroys(
@@ -339,9 +377,10 @@ void LiveRange::insertEndBorrowsAtDestroys(
   }
 }
 
-void LiveRange::convertOwnedGeneralForwardingUsesToGuaranteed() {
+void LiveRange::convertOwnedGeneralForwardingUsesToGuaranteed() && {
   while (!ownershipForwardingUses.empty()) {
-    auto *i = ownershipForwardingUses.pop_back_val()->getUser();
+    auto *i = ownershipForwardingUses.back()->getUser();
+    ownershipForwardingUses = ownershipForwardingUses.drop_back();
 
     // If this is a term inst, just convert all of its incoming values that are
     // owned to be guaranteed.
@@ -402,7 +441,8 @@ void LiveRange::convertToGuaranteedAndRAUW(SILValue newGuaranteedValue,
                                            InstModCallbacks callbacks) && {
   auto *value = cast<SingleValueInstruction>(introducer.value);
   while (!destroyingUses.empty()) {
-    auto *d = destroyingUses.pop_back_val();
+    auto *d = destroyingUses.back();
+    destroyingUses = destroyingUses.drop_back();
     callbacks.deleteInst(d->getUser());
     ++NumEliminatedInsts;
   }
@@ -412,7 +452,7 @@ void LiveRange::convertToGuaranteedAndRAUW(SILValue newGuaranteedValue,
   // Then change all of our guaranteed forwarding insts to have guaranteed
   // ownership kind instead of what ever they previously had (ignoring trivial
   // results);
-  convertOwnedGeneralForwardingUsesToGuaranteed();
+  std::move(*this).convertOwnedGeneralForwardingUsesToGuaranteed();
 }
 
 void LiveRange::convertArgToGuaranteed(DeadEndBlocks &deadEndBlocks,
@@ -429,15 +469,16 @@ void LiveRange::convertArgToGuaranteed(DeadEndBlocks &deadEndBlocks,
 
   // Then eliminate all of the destroys...
   while (!destroyingUses.empty()) {
-    auto *d = destroyingUses.pop_back_val();
+    auto *d = destroyingUses.back();
+    destroyingUses = destroyingUses.drop_back();
     callbacks.deleteInst(d->getUser());
     ++NumEliminatedInsts;
   }
 
   // and change all of our guaranteed forwarding insts to have guaranteed
   // ownership kind instead of what ever they previously had (ignoring trivial
   // results);
-  convertOwnedGeneralForwardingUsesToGuaranteed();
+  std::move(*this).convertOwnedGeneralForwardingUsesToGuaranteed();
 }
 
 LiveRange::HasConsumingUse_t
@@ -1297,8 +1338,9 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
     SmallVector<Operand *, 8> scratchSpace;
     SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
     if (llvm::any_of(borrowScopeIntroducers, [&](BorrowedValue borrowScope) {
-          return !borrowScope.areUsesWithinScope(
-              destroys, scratchSpace, visitedBlocks, getDeadEndBlocks());
+          return !borrowScope.areUsesWithinScope(lr.getAllConsumingUses(),
+                                                 scratchSpace, visitedBlocks,
+                                                 getDeadEndBlocks());
         })) {
       return false;
     }
@@ -1330,12 +1372,11 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
         return false;
       }
 
-      if (llvm::any_of(borrowScopeIntroducers,
-                       [&](BorrowedValue borrowScope) {
-                         return !borrowScope.areUsesWithinScope(
-                             phiArgLR.getDestroyingUses(), scratchSpace,
-                             visitedBlocks, getDeadEndBlocks());
-                       })) {
+      if (llvm::any_of(borrowScopeIntroducers, [&](BorrowedValue borrowScope) {
+            return !borrowScope.areUsesWithinScope(
+                phiArgLR.getAllConsumingUses(), scratchSpace, visitedBlocks,
+                getDeadEndBlocks());
+          })) {
         return false;
       }
     }
@@ -1641,7 +1682,7 @@ class StorageGuaranteesLoadVisitor
     SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
     LinearLifetimeChecker checker(visitedBlocks, ARCOpt.getDeadEndBlocks());
     if (!checker.validateLifetime(access, endScopeUses,
-                                  liveRange.getDestroyingUses())) {
+                                  liveRange.getAllConsumingUses())) {
       // If we fail the linear lifetime check, then just recur:
       return next(access->getOperand());
     }
@@ -1738,8 +1779,8 @@ class StorageGuaranteesLoadVisitor
     LinearLifetimeChecker checker(visitedBlocks, ARCOpt.getDeadEndBlocks());
 
     // Returns true on success. So we invert.
-    bool foundError = !checker.validateLifetime(baseObject, endScopeInsts,
-                                                liveRange.getDestroyingUses());
+    bool foundError = !checker.validateLifetime(
+        baseObject, endScopeInsts, liveRange.getAllConsumingUses());
     return answer(foundError);
   }
 
@@ -1777,7 +1818,7 @@ class StorageGuaranteesLoadVisitor
     // Returns true on success. So we invert.
     bool foundError = !checker.validateLifetime(
         stack, destroyAddrOperands /*consuming users*/,
-        liveRange.getDestroyingUses() /*non consuming users*/);
+        liveRange.getAllConsumingUses() /*non consuming users*/);
     return answer(foundError);
   }
 

Diff for: lib/SILOptimizer/Utils/InstOptUtils.cpp

@@ -58,12 +58,14 @@ swift::createIncrementBefore(SILValue ptr, SILInstruction *insertPt) {
   // If Ptr is refcounted itself, create the strong_retain and
   // return.
   if (ptr->getType().isReferenceCounted(builder.getModule())) {
-    if (ptr->getType().is<UnownedStorageType>())
-      return builder.createUnownedRetain(loc, ptr,
-                                         builder.getDefaultAtomicity());
-    else
-      return builder.createStrongRetain(loc, ptr,
-                                        builder.getDefaultAtomicity());
+#define ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...)            \
+    if (ptr->getType().is<Name##StorageType>())                                \
+      return builder.create##Name##Retain(loc, ptr,                            \
+                                          builder.getDefaultAtomicity());
+#include "swift/AST/ReferenceStorage.def"
+
+    return builder.createStrongRetain(loc, ptr,
+                                      builder.getDefaultAtomicity());
   }
 
   // Otherwise, create the retain_value.
@@ -85,12 +87,14 @@ swift::createDecrementBefore(SILValue ptr, SILInstruction *insertPt) {
 
   // If ptr has reference semantics itself, create a strong_release.
   if (ptr->getType().isReferenceCounted(builder.getModule())) {
-    if (ptr->getType().is<UnownedStorageType>())
-      return builder.createUnownedRelease(loc, ptr,
+#define ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...)            \
+    if (ptr->getType().is<Name##StorageType>())                                \
+      return builder.create##Name##Release(loc, ptr,                           \
                                           builder.getDefaultAtomicity());
-    else
-      return builder.createStrongRelease(loc, ptr,
-                                         builder.getDefaultAtomicity());
+#include "swift/AST/ReferenceStorage.def"
+
+    return builder.createStrongRelease(loc, ptr,
+                                       builder.getDefaultAtomicity());
   }
 
   // Otherwise create a release value.

Diff for: test/SILOptimizer/semantic-arc-opts.sil

@@ -59,6 +59,7 @@ class ClassLet {
   @_hasStorage let aLet: Klass
   @_hasStorage var aVar: Klass
   @_hasStorage let aLetTuple: (Klass, Klass)
+  @_hasStorage let anOptionalLet: FakeOptional<Klass>
 
   @_hasStorage let anotherLet: ClassLet
 }
@@ -2204,3 +2205,31 @@ bb6:
   %9999 = tuple()
   return %9999 : $()
 }
+
+// Make sure that we do not promote the load [copy] to a load_borrow since it
+// has a use outside of the access scope.
+//
+// CHECK-LABEL: sil [ossa] @deadEndBlockDoNotPromote : $@convention(method) (@guaranteed ClassLet) -> () {
+// CHECK: load_borrow
+// CHECK: load [copy]
+// CHECK: } // end sil function 'deadEndBlockDoNotPromote'
+sil [ossa] @deadEndBlockDoNotPromote : $@convention(method) (@guaranteed ClassLet) -> () {
+bb0(%0 : @guaranteed $ClassLet):
+  %4 = ref_element_addr %0 : $ClassLet, #ClassLet.anotherLet
+  %5 = load [copy] %4 : $*ClassLet
+  %6 = begin_borrow %5 : $ClassLet
+  %7 = ref_element_addr %6 : $ClassLet, #ClassLet.anOptionalLet
+  %8 = begin_access [read] [dynamic] %7 : $*FakeOptional<Klass>
+  %9 = load [copy] %8 : $*FakeOptional<Klass>
+  end_access %8 : $*FakeOptional<Klass>
+  end_borrow %6 : $ClassLet
+  destroy_value %5 : $ClassLet
+  switch_enum %9 : $FakeOptional<Klass>, case #FakeOptional.none!enumelt: bb1, case #FakeOptional.some!enumelt: bb2
+
+bb1:
+  %107 = tuple ()
+  return %107 : $()
+
+bb2(%39 : @owned $Klass):
+  unreachable
+}

Diff for: test/api-digester/compare-dump-abi.swift

@@ -5,6 +5,8 @@
 // RUN: %empty-directory(%t.baseline)
 // RUN: %empty-directory(%t.baseline/ABI)
 
+// REQUIRES: rdar62111064
+
 // RUN: %swift -emit-module -o %t.mod1/cake.swiftmodule %S/Inputs/cake_baseline/cake.swift -parse-as-library -enable-library-evolution -I %S/Inputs/APINotesLeft %clang-importer-sdk-nosource -emit-module-source-info -emit-module-source-info-path %t.mod1/cake.swiftsourceinfo 2> %t.compiler-diags
 // RUN: %swift -emit-module -o %t.mod2/cake.swiftmodule %S/Inputs/cake_current/cake.swift -parse-as-library -enable-library-evolution -I %S/Inputs/APINotesRight %clang-importer-sdk-nosource -emit-module-source-info -emit-module-source-info-path %t.mod2/cake.swiftsourceinfo
 // RUN: %api-digester -dump-sdk -module cake -output-dir %t.baseline -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod1 -I %S/Inputs/APINotesLeft -abi
@@ -21,4 +23,4 @@
 // CHECK: cake_current/cake.swift:33:14: error: cake: Class C5 is now without @objc
 // CHECK: cake_current/cake.swift:35:23: error: cake: Func C5.dy_foo() is now with dynamic
 // CHECK: cake_current/cake.swift:39:15: error: cake: Struct C6 is now with @frozen
-// CHECK: cake_current/cake.swift:41:13: error: cake: Enum IceKind is now without @frozen
+// CHECK: cake_current/cake.swift:41:13: error: cake: Enum IceKind is now without @frozen

Diff for: tools/swift-api-digester/swift-api-digester.cpp

@@ -2700,6 +2700,10 @@ static StringRef getBaselineFilename(llvm::Triple Triple) {
     return "appletvos.json";
   else if (Triple.isWatchOS())
     return "watchos.json";
+  else if (Triple.isOSLinux())
+    return "linux.json";
+  else if (Triple.isOSWindows())
+    return "windows.json";
   else {
     llvm::errs() << "Unsupported triple target\n";
     exit(1);