Merge pull request #2049 from swiftwasm/main

[pull] swiftwasm from main

Author: pull[bot]

include/swift/SIL/MemAccessUtils.h

@@ -49,7 +49,7 @@
 ///
 /// For better identification an access base, use
 /// AccessedStorage::compute(). It returns an AccessedStorage value
-/// that identifies the storage location of a memory access. It provides APIs
+/// that identifies the storage of a memory access. It provides APIs
 /// for inspecting type of accessed storage and allows for disambiguation
 /// between different types of storage and different properties within a class.
 ///
@@ -64,7 +64,7 @@
 /// from an enforced begin_access or from any memory operation that is part of a
 /// formal access, then it returns a valid AccessedStorage value. If the memory
 /// operation is not part of a formal access, then it still identifies the
-/// accessed location as a best effort, but the result may be invalid storage.
+/// accessed storage as a best effort, but the result may be invalid storage.
 ///
 ///    An active goal is to require compute() to always return a
 ///    valid AccessedStorage value even for operations that aren't part of a
@@ -77,13 +77,30 @@
 /// make incorrect assumptions about the absence of access to globals or class
 /// properties.
 ///
-/// identifyFormalAccess() is similar to AccessedStorage::compute(), but returns
-/// the formally accessed storage of a begin_access instruction. This must
-/// return a valid AccessedStorage value unless the access has "Unsafe"
-/// enforcement. The formal access location may be nested within an outer
-/// begin_access. For the purpose of exclusivity, nested accesses are considered
-/// distinct formal accesses so they return distinct AccessedStorage values even
-/// though they may access the same memory.
+/// AccessedStorage::computeInScope() returns an AccessedStorage value for the
+/// immediately enclosing access scope. Within a formal access, it always
+/// returns a Nested storage kind, which provides the begin_access marker.
+///
+/// identifyFormalAccess() works like AccessedStorage::computeInScope(), but
+/// finds the storage corresponding to a begin_access marker, rather than an
+/// arbitrary address. This must return a valid AccessedStorage value unless the
+/// access has "Unsafe" enforcement. The given begin_access marker may be nested
+/// within another, outer access scope. For the purpose of exclusivity, nested
+/// accesses are considered distinct formal accesses so they return distinct
+/// AccessedStorage values even though they may access the same memory. This
+/// way, nested accesses do not appear to conflict.
+///
+/// AccessPath identifies both the accessed storage and the path to a specific
+/// storage location within that storage object. See ProgrammersGuide.md and the
+/// class comments below for details. AccessPath::compute() and
+/// AccessPath::computeInScope() mirror the AccessedStorage API.
+/// AccessPath::contains() and AccessPath::mayOverlap() provide efficient
+/// comparison of access paths.
+///
+/// AccessPath::collectUses() provides all reachable uses of the accessed
+/// storage, allowing the selection of Exact, Inner, or Overlapping uses.
+/// visitAccessedStorageUses() and visitAccessPathUses() generalize
+/// handling of all reachable uses for a given storage location.
 ///
 //===----------------------------------------------------------------------===//
 
@@ -1201,6 +1218,10 @@ SILBasicBlock::iterator removeBeginAccess(BeginAccessInst *beginAccess);
 
 namespace swift {
 
+/// Return true if \p svi is a cast that preserves the identity and
+/// reference-counting equivalence of the reference at operand zero.
+bool isRCIdentityPreservingCast(SingleValueInstruction *svi);
+
 /// If \p svi is an access projection, return an address-type operand for the
 /// incoming address.
 ///

lib/SIL/IR/SILType.cpp

@@ -129,6 +129,9 @@ bool SILType::isPointerSizeAndAligned() {
 //
 // TODO: handle casting to a loadable existential by generating
 // init_existential_ref. Until then, only promote to a heap object dest.
+//
+// This cannot allow trivial-to-reference casts, as required by
+// isRCIdentityPreservingCast.
 bool SILType::canRefCast(SILType operTy, SILType resultTy, SILModule &M) {
   auto fromTy = operTy.unwrapOptionalType();
   auto toTy = resultTy.unwrapOptionalType();

lib/SIL/Utils/InstructionUtils.cpp

@@ -12,6 +12,7 @@
 
 #define DEBUG_TYPE "sil-inst-utils"
 #include "swift/SIL/InstructionUtils.h"
+#include "swift/SIL/MemAccessUtils.h"
 #include "swift/AST/SubstitutionMap.h"
 #include "swift/Basic/NullablePtr.h"
 #include "swift/Basic/STLExtras.h"
@@ -64,20 +65,6 @@ SILValue swift::getUnderlyingObjectStopAtMarkDependence(SILValue v) {
   }
 }
 
-static bool isRCIdentityPreservingCast(ValueKind Kind) {
-  switch (Kind) {
-  case ValueKind::UpcastInst:
-  case ValueKind::UncheckedRefCastInst:
-  case ValueKind::UnconditionalCheckedCastInst:
-  case ValueKind::UnconditionalCheckedCastValueInst:
-  case ValueKind::RefToBridgeObjectInst:
-  case ValueKind::BridgeObjectToRefInst:
-    return true;
-  default:
-    return false;
-  }
-}
-
 /// Return the underlying SILValue after stripping off identity SILArguments if
 /// we belong to a BB with one predecessor.
 SILValue swift::stripSinglePredecessorArgs(SILValue V) {
@@ -122,42 +109,39 @@ SILValue swift::stripSinglePredecessorArgs(SILValue V) {
   }
 }
 
-SILValue swift::stripCastsWithoutMarkDependence(SILValue V) {
+SILValue swift::stripCastsWithoutMarkDependence(SILValue v) {
   while (true) {
-    V = stripSinglePredecessorArgs(V);
-
-    auto K = V->getKind();
-    if (isRCIdentityPreservingCast(K)
-        || K == ValueKind::UncheckedTrivialBitCastInst
-        || K == ValueKind::BeginAccessInst
-        || K == ValueKind::EndCOWMutationInst) {
-      V = cast<SingleValueInstruction>(V)->getOperand(0);
-      continue;
+    v = stripSinglePredecessorArgs(v);
+    if (auto *svi = dyn_cast<SingleValueInstruction>(v)) {
+      if (isRCIdentityPreservingCast(svi)
+          || isa<UncheckedTrivialBitCastInst>(v)
+          || isa<BeginAccessInst>(v)
+          || isa<EndCOWMutationInst>(v)) {
+        v = svi->getOperand(0);
+        continue;
+      }
     }
-
-    return V;
+    return v;
   }
 }
 
 SILValue swift::stripCasts(SILValue v) {
   while (true) {
     v = stripSinglePredecessorArgs(v);
-    
-    auto k = v->getKind();
-    if (isRCIdentityPreservingCast(k)
-        || k == ValueKind::UncheckedTrivialBitCastInst
-        || k == ValueKind::MarkDependenceInst
-        || k == ValueKind::BeginAccessInst) {
-      v = cast<SingleValueInstruction>(v)->getOperand(0);
-      continue;
+    if (auto *svi = dyn_cast<SingleValueInstruction>(v)) {
+      if (isRCIdentityPreservingCast(svi)
+          || isa<UncheckedTrivialBitCastInst>(v)
+          || isa<MarkDependenceInst>(v)
+          || isa<BeginAccessInst>(v)) {
+        v = cast<SingleValueInstruction>(v)->getOperand(0);
+        continue;
+      }
     }
-
     SILValue v2 = stripOwnershipInsts(v);
     if (v2 != v) {
       v = v2;
       continue;
     }
-
     return v;
   }
 }

lib/SIL/Utils/MemAccessUtils.cpp

@@ -365,15 +365,38 @@ bool swift::isLetAddress(SILValue address) {
 //                          MARK: FindReferenceRoot
 //===----------------------------------------------------------------------===//
 
+// On some platforms, casting from a metatype to a reference type dynamically
+// allocates a ref-counted box for the metatype. Naturally that is the place
+// where RC-identity begins. Considering the source of such a casts to be
+// RC-identical would confuse ARC optimization, which might eliminate a retain
+// of such an object completely.
+//
+// The SILVerifier checks that none of these operations cast a nontrivial value
+// to a reference except unconditional_checked_cast[_value].
+bool swift::isRCIdentityPreservingCast(SingleValueInstruction *svi) {
+  switch (svi->getKind()) {
+  default:
+    return false;
+  // Ignore ownership casts
+  case SILInstructionKind::CopyValueInst:
+  case SILInstructionKind::BeginBorrowInst:
+  // Ignore class type casts
+  case SILInstructionKind::UpcastInst:
+  case SILInstructionKind::UncheckedRefCastInst:
+  case SILInstructionKind::RefToBridgeObjectInst:
+  case SILInstructionKind::BridgeObjectToRefInst:
+    return true;
+  case SILInstructionKind::UnconditionalCheckedCastInst:
+  case SILInstructionKind::UnconditionalCheckedCastValueInst:
+    // If the source is nontrivial, then this checked cast may actually create a
+    // new object, so its source is not ref-count equivalent.
+    return !svi->getOperand(0)->getType().isTrivial(*svi->getFunction());
+  }
+}
+
 namespace {
 
 // Essentially RC identity where the starting point is already a reference.
-//
-// FIXME: We cannot currently see through type casts for true RC identity,
-// because property indices are not unique within a class hierarchy. Either fix
-// RefElementAddr::getFieldNo so it returns a unique index, or figure out a
-// different way to encode the property's VarDecl. (Note that the lack of a
-// unique property index could be the source of bugs elsewhere).
 class FindReferenceRoot {
   SmallPtrSet<SILPhiArgument *, 4> visitedPhis;
 
@@ -386,18 +409,13 @@ class FindReferenceRoot {
 
 protected:
   // Return an invalid value for a phi with no resolved inputs.
-  //
-  // FIXME: We should be able to see through RC identity like this:
-  //   nextRoot = stripRCIdentityCasts(nextRoot);
   SILValue recursiveFindRoot(SILValue ref) {
-    while (true) {
-      SILValue nextRoot = ref;
-      nextRoot = stripOwnershipInsts(nextRoot);
-      if (nextRoot == ref)
+    while (auto *svi = dyn_cast<SingleValueInstruction>(ref)) {
+      if (!isRCIdentityPreservingCast(svi)) {
         break;
-      ref = nextRoot;
-    }
-
+      }
+      ref = svi->getOperand(0);
+    };
     auto *phi = dyn_cast<SILPhiArgument>(ref);
     if (!phi || !phi->isPhiArgument()) {
       return ref;
@@ -558,8 +576,16 @@ const ValueDecl *AccessedStorage::getDecl(SILValue base) const {
     return global->getDecl();
 
   case Class: {
-    auto *decl = getObject()->getType().getNominalOrBoundGenericNominal();
-    return decl ? getIndexedField(decl, getPropertyIndex()) : nullptr;
+    // The property index is relative to the VarDecl in ref_element_addr, and
+    // can only be reliably determined when the base is avaiable. Otherwise, we
+    // can only make a best effort to extract it from the object type, which
+    // might not even be a class in the case of bridge objects.
+    if (ClassDecl *classDecl =
+            base ? cast<RefElementAddrInst>(base)->getClassDecl()
+                 : getObject()->getType().getClassOrBoundGenericClass()) {
+      return getIndexedField(classDecl, getPropertyIndex());
+    }
+    return nullptr;
   }
   case Tail:
     return nullptr;
@@ -1297,7 +1323,14 @@ void AccessPathDefUseTraversal::followProjection(SingleValueInstruction *svi,
 AccessPathDefUseTraversal::UseKind
 AccessPathDefUseTraversal::visitSingleValueUser(SingleValueInstruction *svi,
                                                 DFSEntry dfs) {
-  if (isAccessedStorageCast(svi)) {
+  if (dfs.isRef()) {
+    if (isRCIdentityPreservingCast(svi)) {
+      pushUsers(svi, dfs);
+      return IgnoredUse;
+    }
+    // 'svi' will be processed below as either RefElementAddrInst,
+    // RefTailAddrInst, or some unknown LeafUse.
+  } else if (isAccessedStorageCast(svi)) {
     pushUsers(svi, dfs);
     return IgnoredUse;
   }

lib/SIL/Verifier/SILVerifier.cpp

@@ -3591,6 +3591,15 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     verifyOpenedArchetype(CI, CI->getType().getASTType());
   }
 
+  // Make sure that opcodes handled by isRCIdentityPreservingCast cannot cast
+  // from a trivial to a reference type. Such a cast may dynamically
+  // instantiate a new reference-counted object.
+  void checkNoTrivialToReferenceCast(SingleValueInstruction *svi) {
+    require(!svi->getOperand(0)->getType().isTrivial(*svi->getFunction())
+            || svi->getType().isTrivial(*svi->getFunction()),
+            "Unexpected trivial-to-reference conversion: ");
+  }
+
   /// Verify if a given type is or contains an opened archetype or dynamic self.
   /// If this is the case, verify that the provided instruction has a type
   /// dependent operand for it.
@@ -3753,6 +3762,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
   void checkUpcastInst(UpcastInst *UI) {
     require(UI->getType() != UI->getOperand()->getType(),
             "can't upcast to same type");
+    checkNoTrivialToReferenceCast(UI);
     if (UI->getType().is<MetatypeType>()) {
       CanType instTy(UI->getType().castTo<MetatypeType>()->getInstanceType());
       require(UI->getOperand()->getType().is<MetatypeType>(),