Cache 'isLet' within AccessedStorage.

Compute 'isLet' from the VarDecl that is available when constructing
AccessedStorage so we don't need to recover the VarDecl for the base
later.

This generally makes more sense and is more efficient, but it will be
necessary when we look past class casts when finding the reference root.

Author: atrick

include/swift/SIL/MemAccessUtils.h

@@ -303,11 +303,11 @@ class AccessedStorage {
 protected:
   // Checking the storage kind is far more common than other fields. Make sure
   // it can be byte load with no shift.
-  static const int ReservedKindBits = 8;
+  static const int ReservedKindBits = 7;
   static_assert(ReservedKindBits >= NumKindBits, "Too many storage kinds.");
 
   static const unsigned InvalidElementIndex =
-      (1 << (32 - ReservedKindBits)) - 1;
+      (1 << (32 - (ReservedKindBits + 1))) - 1;
 
   // Form a bitfield that is effectively a union over any pass-specific data
   // with the fields used within this class as a common prefix.
@@ -327,9 +327,10 @@ class AccessedStorage {
     // elementIndex can overflow while gracefully degrading analysis. For now,
     // reserve an absurd number of bits at a nice alignment boundary, but this
     // can be reduced.
-    SWIFT_INLINE_BITFIELD_BASE(AccessedStorage, 32, kind
-                               : ReservedKindBits,
-                                 elementIndex : 32 - ReservedKindBits);
+    SWIFT_INLINE_BITFIELD_BASE(AccessedStorage, 32,
+                               kind : ReservedKindBits,
+                               isLet : 1,
+                               elementIndex : 32 - (ReservedKindBits + 1));
 
     // Define bits for use in AccessedStorageAnalysis. Each identified storage
     // object is mapped to one instance of this subclass.
@@ -512,7 +513,7 @@ class AccessedStorage {
   }
 
   /// Return true if the given access is on a 'let' lvalue.
-  bool isLetAccess(SILFunction *F) const;
+  bool isLetAccess() const { return Bits.AccessedStorage.isLet; }
 
   /// If this is a uniquely identified formal access, then it cannot
   /// alias with any other uniquely identified access to different storage.
@@ -555,9 +556,12 @@ class AccessedStorage {
   /// Returns the ValueDecl for the underlying storage, if it can be
   /// determined. Otherwise returns null.
   ///
-  /// WARNING: This is not a constant-time operation. It is for diagnostics and
-  /// checking via the ValueDecl if we are processing a `let` variable.
-  const ValueDecl *getDecl() const;
+  /// If \p base is provided, then it must be the accessed base for this
+  /// storage, as passed to the AccessedStorage constructor. What \p base is
+  /// provided, this is guaranteed to return a valid decl for class properties;
+  /// otherwise it is only a best effort based on the type of the object root
+  /// *before* the object is cast to the final accessed reference type.
+  const ValueDecl *getDecl(SILValue base = SILValue()) const;
 
   /// Get all leaf uses of all address, pointer, or box values that have a this
   /// AccessedStorage in common. Return true if all uses were found before
@@ -629,6 +633,8 @@ class AccessedStorage {
     // nested/argument access.
     return false;
   }
+
+  void setLetAccess(SILValue base);
 };
 
 } // end namespace swift

include/swift/SIL/SILInstruction.h

@@ -5822,13 +5822,11 @@ class TupleElementAddrInst
 ///                object, including properties declared in a superclass.
 unsigned getFieldIndex(NominalTypeDecl *decl, VarDecl *property);
 
-/// Get the property for a struct or class by its unique index.
+/// Get the property for a struct or class by its unique index, or nullptr if
+/// the index does not match a property declared in this struct or class or
+/// one its superclasses.
 ///
 /// Precondition: \p decl must be a non-resilient struct or class.
-///
-/// Precondition: \p index must be the index of a stored property
-///               (as returned by getFieldIndex()) which is declared
-///               in \p decl, not in a superclass.
 VarDecl *getIndexedField(NominalTypeDecl *decl, unsigned index);
 
 /// A common base for instructions that require a cached field index.

lib/SIL/IR/SILInstructions.cpp

@@ -1345,15 +1345,23 @@ unsigned swift::getFieldIndex(NominalTypeDecl *decl, VarDecl *field) {
 
 /// Get the property for a struct or class by its unique index.
 VarDecl *swift::getIndexedField(NominalTypeDecl *decl, unsigned index) {
-  if (auto *classDecl = dyn_cast<ClassDecl>(decl)) {
-    for (auto *superDecl = classDecl->getSuperclassDecl(); superDecl != nullptr;
-         superDecl = superDecl->getSuperclassDecl()) {
-      assert(index >= superDecl->getStoredProperties().size()
-             && "field index cannot refer to a superclass field");
-      index -= superDecl->getStoredProperties().size();
+  if (auto *structDecl = dyn_cast<StructDecl>(decl)) {
+    return structDecl->getStoredProperties()[index];
+  }
+  auto *classDecl = cast<ClassDecl>(decl);
+  SmallVector<ClassDecl *, 3> superclasses;
+  for (auto *superDecl = classDecl; superDecl != nullptr;
+       superDecl = superDecl->getSuperclassDecl()) {
+    superclasses.push_back(superDecl);
+  }
+  std::reverse(superclasses.begin(), superclasses.end());
+  for (auto *superDecl : superclasses) {
+    if (index < superDecl->getStoredProperties().size()) {
+      return superDecl->getStoredProperties()[index];
     }
+    index -= superDecl->getStoredProperties().size();
   }
-  return decl->getStoredProperties()[index];
+  return nullptr;
 }
 
 unsigned FieldIndexCacheBase::cacheFieldIndex() {

lib/SIL/Utils/MemAccessUtils.cpp

@@ -452,9 +452,9 @@ SILGlobalVariable *getReferencedGlobal(SILInstruction *inst) {
 constexpr unsigned AccessedStorage::TailIndex;
 
 AccessedStorage::AccessedStorage(SILValue base, Kind kind) {
+  // For kind==Unidentified, base may be an invalid empty or tombstone value.
   assert(base && "invalid storage base");
   initKind(kind);
-
   switch (kind) {
   case Box:
     assert(isa<AllocBoxInst>(base));
@@ -505,6 +505,7 @@ AccessedStorage::AccessedStorage(SILValue base, Kind kind) {
     break;
   }
   }
+  setLetAccess(base);
 }
 
 void AccessedStorage::visitRoots(
@@ -528,21 +529,24 @@ void AccessedStorage::visitRoots(
   }
 }
 
-// Return true if the given access is on a 'let' lvalue.
-bool AccessedStorage::isLetAccess(SILFunction *F) const {
-  if (auto *decl = dyn_cast_or_null<VarDecl>(getDecl()))
-    return decl->isLet();
-
+// Set 'isLet' to true if this storage can be determined to be a 'let' variable.
+//
+// \p base must be the access base for this storage, as passed to the
+// AccessedStorage constructor.
+void AccessedStorage::setLetAccess(SILValue base) {
   // It's unclear whether a global will ever be missing it's varDecl, but
   // technically we only preserve it for debug info. So if we don't have a decl,
   // check the flag on SILGlobalVariable, which is guaranteed valid,
-  if (getKind() == AccessedStorage::Global)
-    return getGlobal()->isLet();
-
-  return false;
+  if (getKind() == AccessedStorage::Global) {
+    Bits.AccessedStorage.isLet = getGlobal()->isLet();
+    return;
+  }
+  if (auto *decl = dyn_cast_or_null<VarDecl>(getDecl(base))) {
+    Bits.AccessedStorage.isLet = decl->isLet();
+  }
 }
 
-const ValueDecl *AccessedStorage::getDecl() const {
+const ValueDecl *AccessedStorage::getDecl(SILValue base) const {
   switch (getKind()) {
   case Box:
     return cast<AllocBoxInst>(value)->getLoc().getAsASTNode<VarDecl>();
@@ -555,7 +559,7 @@ const ValueDecl *AccessedStorage::getDecl() const {
 
   case Class: {
     auto *decl = getObject()->getType().getNominalOrBoundGenericNominal();
-    return getIndexedField(decl, getPropertyIndex());
+    return decl ? getIndexedField(decl, getPropertyIndex()) : nullptr;
   }
   case Tail:
     return nullptr;
@@ -621,8 +625,10 @@ void AccessedStorage::print(raw_ostream &os) const {
     break;
   case Class:
     os << getObject();
-    os << "  Field: ";
-    getDecl()->print(os);
+    if (auto *decl = getDecl()) {
+      os << "  Field: ";
+      decl->print(os);
+    }
     os << " Index: " << getPropertyIndex() << "\n";
     break;
   case Tail:
@@ -1655,7 +1661,7 @@ bool swift::isPossibleFormalAccessBase(const AccessedStorage &storage,
   // Additional checks that apply to anything that may fall through.
 
   // Immutable values are only accessed for initialization.
-  if (storage.isLetAccess(F))
+  if (storage.isLetAccess())
     return false;
 
   return true;

lib/SIL/Verifier/LoadBorrowInvalidationChecker.cpp

@@ -466,9 +466,9 @@ bool LoadBorrowNeverInvalidatedAnalysis::isNeverInvalidated(
   }
 
   // If we have a let address, then we are already done.
-  if (storage.isLetAccess(lbi->getFunction()))
+  if (storage.isLetAccess()) {
     return true;
-
+  }
   // At this point, we know that we /may/ have writes. Now we go through various
   // cases to try and exhaustively identify if those writes overlap with our
   // load_borrow.

lib/SILOptimizer/SemanticARC/LoadCopyToLoadBorrowOpt.cpp

@@ -193,7 +193,7 @@ class StorageGuaranteesLoadVisitor
 
   void visitGlobalAccess(SILValue global) {
     return answer(
-        !AccessedStorage(global, AccessedStorage::Global).isLetAccess(&ctx.fn));
+        !AccessedStorage(global, AccessedStorage::Global).isLetAccess());
   }
 
   void visitClassAccess(RefElementAddrInst *field) {