Replace AccessedStorage projection with an index.

atrick · atrick · commit c1bda8f0908d · 2019-05-14T12:44:46.000-07:00
Further simplify AccessedStorage. Shrink it to two words. Remove the
Projection abstraction and streamline the projection logic.
diff --git a/include/swift/SIL/MemAccessUtils.h b/include/swift/SIL/MemAccessUtils.h
@@ -15,8 +15,9 @@
 #ifndef SWIFT_SIL_MEMACCESSUTILS_H
 #define SWIFT_SIL_MEMACCESSUTILS_H
 
-#include "swift/SIL/Projection.h"
 #include "swift/SIL/InstructionUtils.h"
+#include "swift/SIL/SILArgument.h"
+#include "swift/SIL/SILBasicBlock.h"
 #include "swift/SIL/SILInstruction.h"
 #include "llvm/ADT/DenseMap.h"
 
@@ -28,31 +29,6 @@ inline bool accessKindMayConflict(SILAccessKind a, SILAccessKind b) {
   return !(a == SILAccessKind::Read && b == SILAccessKind::Read);
 }
 
-/// Represents the identity of a stored class property as a combination
-/// of a base and projection.
-class ObjectProjection {
-  SILValue object;
-  Projection proj;
-
-public:
-  ObjectProjection(SILValue object, const Projection &projection)
-      : object(object), proj(projection) {
-    assert(object->getType().isObject());
-  }
-
-  SILValue getObject() const { return object; }
-
-  const Projection &getProjection() const { return proj; }
-
-  bool operator==(const ObjectProjection &other) const {
-    return object == other.object && proj == other.proj;
-  }
-
-  bool operator!=(const ObjectProjection &other) const {
-    return object != other.object || proj != other.proj;
-  }
-};
-
 /// Represents the identity of a storage object being accessed.
 ///
 /// AccessedStorage may be one of several kinds of "identified" storage
@@ -107,7 +83,23 @@ class AccessedStorage {
   /// of access base. Otherwise, return Unidentified.
   static AccessedStorage::Kind classify(SILValue base);
 
+  /// Directly create an AccessedStorage for class property access.
+  static AccessedStorage forClass(SILValue object, unsigned propertyIndex) {
+    AccessedStorage storage;
+    storage.initKind(Class, propertyIndex);
+    storage.value = object;
+    return storage;
+  }
+
 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_assert(ReservedKindBits >= NumKindBits, "Too many storage kinds.");
+
+  static const unsigned InvalidElementIndex =
+      (1 << (32 - ReservedKindBits)) - 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.
   //
@@ -122,9 +114,13 @@ class AccessedStorage {
   // each begin_access to its storage object, unique access index, and summary
   // info for that access.
   union {
-    uint64_t OpaqueBits;
-    SWIFT_INLINE_BITFIELD_BASE(AccessedStorage, bitmax(NumKindBits, 8),
-                               Kind : bitmax(NumKindBits, 8));
+    uint64_t opaqueBits;
+    // 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);
 
     // Define bits for use in AccessedStorageAnalysis. Each identified storage
     // object is mapped to one instance of this subclass.
@@ -157,59 +153,67 @@ class AccessedStorage {
 
 private:
   union {
+    // For non-class storage, 'value' is the access base. For class storage
+    // 'value' is the object base, where the access base is the class' stored
+    // property.
     SILValue value;
-    unsigned paramIndex;
     SILGlobalVariable *global;
-    ObjectProjection objProj;
   };
 
-  void initKind(Kind k) {
-    Bits.OpaqueBits = 0;
-    Bits.AccessedStorage.Kind = k;
+  void initKind(Kind k, unsigned elementIndex = InvalidElementIndex) {
+    Bits.opaqueBits = 0;
+    Bits.AccessedStorage.kind = k;
+    Bits.AccessedStorage.elementIndex = elementIndex;
+  }
+
+  unsigned getElementIndex() const { return Bits.AccessedStorage.elementIndex; }
+  void setElementIndex(unsigned elementIndex) {
+    Bits.AccessedStorage.elementIndex = elementIndex;
   }
 
 public:
   AccessedStorage() : value() { initKind(Unidentified); }
 
   AccessedStorage(SILValue base, Kind kind);
 
-  AccessedStorage(SILValue object, Projection projection)
-      : objProj(object, projection) {
-    initKind(Class);
-  }
-
   // Return true if this is a valid storage object.
   operator bool() const { return getKind() != Unidentified || value; }
 
-  Kind getKind() const { return static_cast<Kind>(Bits.AccessedStorage.Kind); }
+  Kind getKind() const { return static_cast<Kind>(Bits.AccessedStorage.kind); }
 
   // Clear any bits reserved for subclass data. Useful for up-casting back to
   // the base class.
-  void resetSubclassData() { initKind(getKind()); }
+  void resetSubclassData() {
+    initKind(getKind(), Bits.AccessedStorage.elementIndex);
+  }
 
   SILValue getValue() const {
-    assert(getKind() != Argument && getKind() != Global && getKind() != Class);
+    assert(getKind() != Global && getKind() != Class);
     return value;
   }
 
   unsigned getParamIndex() const {
     assert(getKind() == Argument);
-    return paramIndex;
+    return getElementIndex();
   }
 
-  SILArgument *getArgument(SILFunction *F) const {
+  SILArgument *getArgument() const {
     assert(getKind() == Argument);
-    return F->getArgument(paramIndex);
+    return cast<SILArgument>(value);
   }
 
   SILGlobalVariable *getGlobal() const {
     assert(getKind() == Global);
     return global;
   }
 
-  const ObjectProjection &getObjectProjection() const {
+  SILValue getObject() const {
     assert(getKind() == Class);
-    return objProj;
+    return value;
+  }
+  unsigned getPropertyIndex() const {
+    assert(getKind() == Class);
+    return getElementIndex();
   }
 
   /// Return true if the given storage objects have identical storage locations.
@@ -223,16 +227,16 @@ class AccessedStorage {
     switch (getKind()) {
     case Box:
     case Stack:
+    case Argument:
     case Yield:
     case Nested:
     case Unidentified:
       return value == other.value;
-    case Argument:
-      return paramIndex == other.paramIndex;
     case Global:
       return global == other.global;
     case Class:
-      return objProj == other.objProj;
+      return value == other.value
+             && getElementIndex() == other.getElementIndex();
     }
   }
 
@@ -286,19 +290,19 @@ class AccessedStorage {
     // Classes are not uniquely identified by their base. However, if the
     // underling objects have identical types and distinct property indices then
     // they are distinct storage locations.
-    auto &proj = getObjectProjection();
-    auto &otherProj = other.getObjectProjection();
-    if (proj.getObject()->getType() == otherProj.getObject()->getType()
-        && proj.getProjection() != otherProj.getProjection()) {
+    if (getObject()->getType() == other.getObject()->getType()
+        && getPropertyIndex() != other.getPropertyIndex()) {
       return true;
     }
     return false;
   }
 
   /// Returns the ValueDecl for the underlying storage, if it can be
-  /// determined. Otherwise returns null. For diagnostics and checking via the
-  /// ValueDecl if we are processing a `let` variable.
-  const ValueDecl *getDecl(SILFunction *F) const;
+  /// 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;
 
   void print(raw_ostream &os) const;
   void dump() const;
@@ -343,8 +347,8 @@ template <> struct DenseMapInfo<swift::AccessedStorage> {
     case swift::AccessedStorage::Global:
       return DenseMapInfo<void *>::getHashValue(storage.getGlobal());
     case swift::AccessedStorage::Class: {
-      const swift::ObjectProjection &P = storage.getObjectProjection();
-      return llvm::hash_combine(P.getObject(), P.getProjection());
+      return llvm::hash_combine(storage.getObject(),
+                                storage.getPropertyIndex());
     }
     }
     llvm_unreachable("Unhandled AccessedStorageKind");
diff --git a/lib/SIL/MemAccessUtils.cpp b/lib/SIL/MemAccessUtils.cpp
@@ -13,7 +13,9 @@
 #define DEBUG_TYPE "sil-access-utils"
 
 #include "swift/SIL/MemAccessUtils.h"
+#include "swift/SIL/ApplySite.h"
 #include "swift/SIL/SILGlobalVariable.h"
+#include "swift/SIL/SILModule.h"
 #include "swift/SIL/SILUndef.h"
 
 using namespace swift;
@@ -81,7 +83,8 @@ AccessedStorage::AccessedStorage(SILValue base, Kind kind) {
     value = base;
     break;
   case Argument:
-    paramIndex = cast<SILFunctionArgument>(base)->getIndex();
+    value = base;
+    setElementIndex(cast<SILFunctionArgument>(base)->getIndex());
     break;
   case Global:
     if (auto *GAI = dyn_cast<GlobalAddrInst>(base))
@@ -105,13 +108,13 @@ AccessedStorage::AccessedStorage(SILValue base, Kind kind) {
     // dynamically checked, and static analysis will be sufficiently
     // conservative given that classes are not "uniquely identified".
     auto *REA = cast<RefElementAddrInst>(base);
-    SILValue Object = stripBorrow(REA->getOperand());
-    objProj = ObjectProjection(Object, Projection(REA));
+    value = stripBorrow(REA->getOperand());
+    setElementIndex(REA->getFieldNo());
   }
   }
 }
 
-const ValueDecl *AccessedStorage::getDecl(SILFunction *F) const {
+const ValueDecl *AccessedStorage::getDecl() const {
   switch (getKind()) {
   case Box:
     return cast<AllocBoxInst>(value)->getLoc().getAsASTNode<VarDecl>();
@@ -122,11 +125,12 @@ const ValueDecl *AccessedStorage::getDecl(SILFunction *F) const {
   case Global:
     return global->getDecl();
 
-  case Class:
-    return objProj.getProjection().getVarDecl(objProj.getObject()->getType());
-
+  case Class: {
+    auto *decl = getObject()->getType().getNominalOrBoundGenericNominal();
+    return *std::next(decl->getStoredProperties().begin(), getPropertyIndex());
+  }
   case Argument:
-    return getArgument(F)->getDecl();
+    return getArgument()->getDecl();
 
   case Yield:
     return nullptr;
@@ -173,15 +177,17 @@ void AccessedStorage::print(raw_ostream &os) const {
     os << value;
     break;
   case Argument:
-    os << "index: " << paramIndex << "\n";
+    os << value;
     break;
   case Global:
     os << *global;
     break;
   case Class:
-    os << objProj.getObject() << "  ";
-    objProj.getProjection().print(os, objProj.getObject()->getType());
-    os << "\n";
+    os << getObject();
+    os << "  Field: ";
+    getDecl()->print(os);
+    os << " Index: " << getPropertyIndex() << "\n";
+    break;
   }
 }
 
@@ -457,7 +463,7 @@ AccessedStorage swift::findAccessedStorageNonNested(SILValue sourceAddr) {
 
 // Return true if the given access is on a 'let' lvalue.
 static bool isLetAccess(const AccessedStorage &storage, SILFunction *F) {
-  if (auto *decl = dyn_cast_or_null<VarDecl>(storage.getDecl(F)))
+  if (auto *decl = dyn_cast_or_null<VarDecl>(storage.getDecl()))
     return decl->isLet();
 
   // It's unclear whether a global will ever be missing it's varDecl, but
diff --git a/lib/SIL/SILVerifier.cpp b/lib/SIL/SILVerifier.cpp
@@ -22,6 +22,8 @@
 #include "swift/AST/Types.h"
 #include "swift/Basic/Range.h"
 #include "swift/ClangImporter/ClangModule.h"
+#include "swift/SIL/ApplySite.h"
+#include "swift/SIL/BasicBlockUtils.h"
 #include "swift/SIL/DebugUtils.h"
 #include "swift/SIL/Dominance.h"
 #include "swift/SIL/DynamicCasts.h"
@@ -35,7 +37,6 @@
 #include "swift/SIL/SILVTable.h"
 #include "swift/SIL/SILVTableVisitor.h"
 #include "swift/SIL/SILVisitor.h"
-#include "swift/SIL/BasicBlockUtils.h"
 #include "swift/SIL/TypeLowering.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/PostOrderIterator.h"
diff --git a/lib/SILOptimizer/Analysis/AccessedStorageAnalysis.cpp b/lib/SILOptimizer/Analysis/AccessedStorageAnalysis.cpp
@@ -234,13 +234,15 @@ transformCalleeStorage(const StorageAccessInfo &storage,
   case AccessedStorage::Class: {
     // If the object's value is an argument, translate it into a value on the
     // caller side.
-    SILValue obj = storage.getObjectProjection().getObject();
+    SILValue obj = storage.getObject();
     if (auto *arg = dyn_cast<SILFunctionArgument>(obj)) {
       SILValue argVal = getCallerArg(fullApply, arg->getIndex());
       if (argVal) {
-        auto &proj = storage.getObjectProjection().getProjection();
-        // Remap the argument source value and inherit the old storage info.
-        return StorageAccessInfo(AccessedStorage(argVal, proj), storage);
+        unsigned idx = storage.getPropertyIndex();
+        // Remap this storage info. The argument source value is now the new
+        // object. The old storage info is inherited.
+        return StorageAccessInfo(AccessedStorage::forClass(argVal, idx),
+                                 storage);
       }
     }
     // Otherwise, continue to reference the value in the callee because we don't
diff --git a/lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp b/lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp
@@ -542,7 +542,7 @@ static void diagnoseExclusivityViolation(const ConflictingAccess &Violation,
   unsigned AccessKindForMain =
       static_cast<unsigned>(MainAccess.getAccessKind());
 
-  if (const ValueDecl *VD = Storage.getDecl(F)) {
+  if (const ValueDecl *VD = Storage.getDecl()) {
     // We have a declaration, so mention the identifier in the diagnostic.
     SILType BaseType = FirstAccess.getInstruction()->getType().getAddressType();
     SILModule &M = FirstAccess.getInstruction()->getModule();
diff --git a/lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp b/lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp
@@ -353,7 +353,7 @@ bool SemanticARCOptVisitor::visitCopyValueInst(CopyValueInst *cvi) {
 // If there is 2+ writes, we can not optimize = (.
 
 bool mayFunctionMutateArgument(const AccessedStorage &storage, SILFunction &f) {
-  auto *arg = cast<SILFunctionArgument>(storage.getArgument(&f));
+  auto *arg = cast<SILFunctionArgument>(storage.getArgument());
 
   // Then check if we have an in_guaranteed argument. In this case, we can
   // always optimize load [copy] from this.
diff --git a/lib/SILOptimizer/Transforms/AccessEnforcementWMO.cpp b/lib/SILOptimizer/Transforms/AccessEnforcementWMO.cpp
diff --git a/test/SILOptimizer/accessed_storage_analysis.sil b/test/SILOptimizer/accessed_storage_analysis.sil
