Merge pull request swiftlang#24458 from atrick/clean-accessed-storage

Remove macros and visitors from AccessedStorage.

Author: atrick

include/swift/SIL/AccessedStorage.def

@@ -106,11 +106,15 @@ class AccessedStorage {
   /// Enumerate over all valid begin_access bases. Clients can use a covered
   /// switch to warn if findAccessedAddressBase ever adds a case.
   enum Kind : uint8_t {
-#define ACCESSED_STORAGE(Name) Name,
-#define ACCESSED_STORAGE_RANGE(Name, Start, End)                               \
-  First_##Name = Start, Last_##Name = End,
-#include "swift/SIL/AccessedStorage.def"
-    NumKindBits = countBitsUsed(unsigned(Last_AccessedStorageKind))
+    Box,
+    Stack,
+    Global,
+    Class,
+    Argument,
+    Yield,
+    Nested,
+    Unidentified,
+    NumKindBits = countBitsUsed(static_cast<unsigned>(Unidentified))
   };
 
   static const char *getKindName(Kind k);
@@ -322,26 +326,6 @@ class AccessedStorage {
   bool operator==(const AccessedStorage &) const = delete;
   bool operator!=(const AccessedStorage &) const = delete;
 };
-
-template <class ImplTy, class ResultTy = void, typename... ArgTys>
-class AccessedStorageVisitor {
-  ImplTy &asImpl() { return static_cast<ImplTy &>(*this); }
-
-public:
-#define ACCESSED_STORAGE(Name)                                                 \
-  ResultTy visit##Name(const AccessedStorage &storage, ArgTys &&... args);
-#include "swift/SIL/AccessedStorage.def"
-
-  ResultTy visit(const AccessedStorage &storage, ArgTys &&... args) {
-    switch (storage.getKind()) {
-#define ACCESSED_STORAGE(Name)                                                 \
-  case AccessedStorage::Name:                                                  \
-    return asImpl().visit##Name(storage, std::forward<ArgTys>(args)...);
-#include "swift/SIL/AccessedStorage.def"
-    }
-  }
-};
-
 } // end namespace swift
 
 namespace llvm {

include/swift/SIL/MemAccessUtils.h

@@ -140,10 +140,22 @@ const ValueDecl *AccessedStorage::getDecl(SILFunction *F) const {
 
 const char *AccessedStorage::getKindName(AccessedStorage::Kind k) {
   switch (k) {
-#define ACCESSED_STORAGE(NAME)                                                 \
-  case AccessedStorage::NAME:                                                  \
-    return #NAME;
-#include "swift/SIL/AccessedStorage.def"
+  case Box:
+    return "Box";
+  case Stack:
+    return "Stack";
+  case Nested:
+    return "Nested";
+  case Unidentified:
+    return "Unidentified";
+  case Argument:
+    return "Argument";
+  case Yield:
+    return "Yield";
+  case Global:
+    return "Global";
+  case Class:
+    return "Class";
   }
   llvm_unreachable("unhandled kind");
 }

lib/SIL/MemAccessUtils.cpp

@@ -339,56 +339,29 @@ bool SemanticARCOptVisitor::visitCopyValueInst(CopyValueInst *cvi) {
 //                         load [copy] Optimizations
 //===----------------------------------------------------------------------===//
 
-/// A flow insensitive analysis that tells the load [copy] analysis if the
-/// storage has 0, 1, >1 writes to it.
-///
-/// In the case of 0 writes, we return CanOptimizeLoadCopyResult::Always.
-///
-/// In the case of 1 write, we return OnlyIfStorageIsLocal. We are taking
-/// advantage of definite initialization implying that an alloc_stack must be
-/// written to once before any loads from the memory location. Thus if we are
-/// local and see 1 write, we can still change to load_borrow if all other uses
-/// check out.
-///
-/// If there is 2+ writes, we can not optimize = (.
-namespace {
-
-struct CanOptimizeLoadCopyFromAccessVisitor
-    : AccessedStorageVisitor<CanOptimizeLoadCopyFromAccessVisitor, bool> {
-  SILFunction &f;
-
-  CanOptimizeLoadCopyFromAccessVisitor(SILFunction &f) : f(f) {}
-
-  // Stubs
-  bool visitBox(const AccessedStorage &boxStorage) { return false; }
-  bool visitStack(const AccessedStorage &stackStorage) { return false; }
-  bool visitGlobal(const AccessedStorage &globalStorage) { return false; }
-  bool visitClass(const AccessedStorage &classStorage) { return false; }
-  bool visitYield(const AccessedStorage &yieldStorage) { return false; }
-  bool visitUnidentified(const AccessedStorage &unidentifiedStorage) {
-    return false;
-  }
-  bool visitNested(const AccessedStorage &nested) {
-    llvm_unreachable("Visitor should never see nested since we lookup our "
-                     "address storage using lookup non nested");
-  }
-
-  bool visitArgument(const AccessedStorage &argumentStorage);
-};
-
-} // namespace
+// A flow insensitive analysis that tells the load [copy] analysis if the
+// storage has 0, 1, >1 writes to it.
+//
+// In the case of 0 writes, we return CanOptimizeLoadCopyResult::Always.
+//
+// In the case of 1 write, we return OnlyIfStorageIsLocal. We are taking
+// advantage of definite initialization implying that an alloc_stack must be
+// written to once before any loads from the memory location. Thus if we are
+// local and see 1 write, we can still change to load_borrow if all other uses
+// check out.
+//
+// If there is 2+ writes, we can not optimize = (.
 
-bool CanOptimizeLoadCopyFromAccessVisitor::visitArgument(
-    const AccessedStorage &storage) {
+bool mayFunctionMutateArgument(const AccessedStorage &storage, SILFunction &f) {
   auto *arg = cast<SILFunctionArgument>(storage.getArgument(&f));
 
   // Then check if we have an in_guaranteed argument. In this case, we can
   // always optimize load [copy] from this.
   if (arg->hasConvention(SILArgumentConvention::Indirect_In_Guaranteed))
-    return true;
+    return false;
 
   // For now just return false.
-  return false;
+  return true;
 }
 
 static bool isWrittenTo(SILFunction &f, SILValue value) {
@@ -402,7 +375,19 @@ static bool isWrittenTo(SILFunction &f, SILValue value) {
   // way (ignoring stores that are obviously the only initializer to
   // memory). We have to do this since load_borrow assumes that the
   // underlying memory is never written to.
-  return !CanOptimizeLoadCopyFromAccessVisitor(f).visit(storage);
+  switch (storage.getKind()) {
+  case AccessedStorage::Box:
+  case AccessedStorage::Stack:
+  case AccessedStorage::Global:
+  case AccessedStorage::Class:
+  case AccessedStorage::Yield:
+  case AccessedStorage::Nested:
+  case AccessedStorage::Unidentified:
+    return true;
+
+  case AccessedStorage::Argument:
+    return mayFunctionMutateArgument(storage, f);
+  }
 }
 
 // Convert a load [copy] from unique storage [read] that has all uses that can