Simplify accessed storage; remove RefElementAddr

include/swift/SIL/MemAccessUtils.h

@@ -29,43 +29,27 @@ inline bool accessKindMayConflict(SILAccessKind a, SILAccessKind b) {
 }
 
 /// Represents the identity of a stored class property as a combination
-/// of a base, projection and projection path.
-/// we pre-compute the base and projection path, even though we can
-/// lazily do so, because it is more expensive otherwise
-/// We lazily compute the projection path,
-/// In the rare occasions we need it, because of Its destructor and
-/// its non-trivial copy constructor
+/// of a base and projection.
 class ObjectProjection {
   SILValue object;
-  const RefElementAddrInst *REA;
   Projection proj;
 
 public:
-  ObjectProjection(const RefElementAddrInst *REA)
-      : object(stripBorrow(REA->getOperand())), REA(REA),
-        proj(Projection(REA)) {}
-
-  ObjectProjection(SILValue object, const RefElementAddrInst *REA)
-      : object(object), REA(REA), proj(Projection(REA)) {}
-
-  const RefElementAddrInst *getInstr() const { return REA; }
+  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; }
 
-  const Optional<ProjectionPath> getProjectionPath() const {
-    return ProjectionPath::getProjectionPath(stripBorrow(REA->getOperand()),
-                                             REA);
-  }
-
   bool operator==(const ObjectProjection &other) const {
-    return getObject() == other.getObject() &&
-           getProjection() == other.getProjection();
+    return object == other.object && proj == other.proj;
   }
 
   bool operator!=(const ObjectProjection &other) const {
-    return !operator==(other);
+    return object != other.object || proj != other.proj;
   }
 };
 
@@ -189,8 +173,8 @@ class AccessedStorage {
 
   AccessedStorage(SILValue base, Kind kind);
 
-  AccessedStorage(SILValue object, const RefElementAddrInst *REA)
-      : objProj(object, REA) {
+  AccessedStorage(SILValue object, Projection projection)
+      : objProj(object, projection) {
     initKind(Class);
   }
 
@@ -295,20 +279,20 @@ class AccessedStorage {
     if (isUniquelyIdentified() && other.isUniquelyIdentified()) {
       return !hasIdenticalBase(other);
     }
-    if (getKind() != Class || other.getKind() != Class) {
-      return false;
-    }
-    if (getObjectProjection().getProjection() ==
-        other.getObjectProjection().getProjection()) {
-      return false;
-    }
-    auto projPath = getObjectProjection().getProjectionPath();
-    auto otherProjPath = other.getObjectProjection().getProjectionPath();
-    if (!projPath.hasValue() || !otherProjPath.hasValue()) {
+    if (getKind() != Class || other.getKind() != Class)
+      // At least one side is an Argument or Yield, or is unidentified.
       return false;
+
+    // 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()) {
+      return true;
     }
-    return projPath.getValue().hasNonEmptySymmetricDifference(
-        otherProjPath.getValue());
+    return false;
   }
 
   /// Returns the ValueDecl for the underlying storage, if it can be

lib/SIL/MemAccessUtils.cpp

@@ -101,10 +101,12 @@ AccessedStorage::AccessedStorage(SILValue base, Kind kind) {
   case Class: {
     // Do a best-effort to find the identity of the object being projected
     // from. It is OK to be unsound here (i.e. miss when two ref_element_addrs
-    // actually refer the same address) because these will be dynamically
-    // checked.
+    // actually refer the same address) because these addresses will be
+    // dynamically checked, and static analysis will be sufficiently
+    // conservative given that classes are not "uniquely identified".
     auto *REA = cast<RefElementAddrInst>(base);
-    objProj = ObjectProjection(REA);
+    SILValue Object = stripBorrow(REA->getOperand());
+    objProj = ObjectProjection(Object, Projection(REA));
   }
   }
 }

lib/SILOptimizer/Analysis/AccessedStorageAnalysis.cpp

@@ -238,9 +238,9 @@ transformCalleeStorage(const StorageAccessInfo &storage,
     if (auto *arg = dyn_cast<SILFunctionArgument>(obj)) {
       SILValue argVal = getCallerArg(fullApply, arg->getIndex());
       if (argVal) {
-        auto *instr = storage.getObjectProjection().getInstr();
+        auto &proj = storage.getObjectProjection().getProjection();
         // Remap the argument source value and inherit the old storage info.
-        return StorageAccessInfo(AccessedStorage(argVal, instr), storage);
+        return StorageAccessInfo(AccessedStorage(argVal, proj), storage);
       }
     }
     // Otherwise, continue to reference the value in the callee because we don't

lib/SILOptimizer/LoopTransforms/LICM.cpp

@@ -347,7 +347,7 @@ hoistSpecialInstruction(std::unique_ptr<LoopNestSummary> &LoopSummary,
           llvm_unreachable("LICM: Could not perform must-sink instruction");
         }
       }
-      LLVM_DEBUG(llvm::errs() << " Successfully hosited and sank pair\n");
+      LLVM_DEBUG(llvm::errs() << " Successfully hoisted and sank pair\n");
     } else {
       LLVM_DEBUG(llvm::dbgs() << "Hoisted RefElementAddr "
                               << *static_cast<RefElementAddrInst *>(Inst));

test/SILOptimizer/access_enforcement_opts.sil

@@ -1352,18 +1352,17 @@ class RefElemClass {
   init()
 }
 
-// Merge access overlapping scopes. Scope nesting does not need to be
-// preserved unless the nested accesses are to identical storage.
+// Merge access overlapping scopes.
 //
 // CHECK-LABEL: sil @ref_elem_c : $@convention(thin) (RefElemClass) -> () {
 // CHECK: [[GLOBAL:%.*]] = global_addr @globalX : $*X
-// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
+// CHECK-NEXT: [[BEGIN:%.*]] =  begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [[BEGIN]] : $*X
 // CHECK-NEXT: end_access [[BEGIN]] : $*X
 // CHECK-NEXT: [[REFX:%.*]] = ref_element_addr %0 : $RefElemClass, #RefElemClass.x
 // CHECK-NEXT: [[REFY:%.*]] = ref_element_addr %0 : $RefElemClass, #RefElemClass.y
 // CHECK-NEXT: [[BEGINX:%.*]] = begin_access [modify] [dynamic] [[REFX]] : $*BitfieldOne
-// CHECK: [[BEGINY:%.*]] = begin_access [modify] [dynamic] [[REFY]] : $*Int32
+// CHECK: [[BEGINY:%.*]] = begin_access [modify] [dynamic] [no_nested_conflict] [[REFY]] : $*Int32
 // CHECK: end_access [[BEGINX]] : $*BitfieldOne
 // CHECK-NEXT: end_access [[BEGINY]] : $*Int32
 // CHECK-LABEL: } // end sil function 'ref_elem_c'

test/SILOptimizer/licm_exclusivity.swift

@@ -1,16 +1,16 @@
-// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil -Xllvm -debug-only=sil-licm -primary-file %s 2>&1 | %FileCheck %s --check-prefix=TEST1
-// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil -Xllvm -debug-only=sil-licm  -primary-file %s 2>&1 | %FileCheck %s --check-prefix=TEST2
+// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil -Xllvm -debug-only=sil-licm -primary-file %s 2>&1 | %FileCheck %s --check-prefix=TESTLICM
+// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil -Xllvm -debug-only=sil-licm -primary-file %s 2>&1 | %FileCheck %s --check-prefix=TESTLICM2
 // RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil  -primary-file %s | %FileCheck %s --check-prefix=TESTSIL
-// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil -Xllvm -debug-only=sil-licm  -whole-module-optimization %s 2>&1 | %FileCheck %s --check-prefix=TEST3
-// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil  -whole-module-optimization %s | %FileCheck %s --check-prefix=TESTSIL2
+// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil -Xllvm -debug-only=sil-licm -whole-module-optimization %s 2>&1 | %FileCheck %s --check-prefix=TESTLICMWMO
+// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil  -whole-module-optimization %s | %FileCheck %s --check-prefix=TESTSILWMO
 
 // REQUIRES: optimized_stdlib,asserts
 // REQUIRES: PTRSIZE=64
 
-// TEST1-LABEL: Processing loops in {{.*}}run_ReversedArray{{.*}}
-// TEST1: Hoist and Sink pairs attempt
-// TEST1: Hoisted
-// TEST1: Successfully hosited and sank pair
+// TESTLICM-LABEL: Processing loops in {{.*}}run_ReversedArray{{.*}}
+// TESTLICM: Hoist and Sink pairs attempt
+// TESTLICM: Hoisted
+// TESTLICM: Successfully hoisted and sank pair
 
 // TESTSIL-LABEL: sil hidden @$s16licm_exclusivity17run_ReversedArrayyySiF : $@convention(thin) (Int) -> () {
 // TESTSIL: bb
@@ -32,9 +32,9 @@ func run_ReversedArray(_ N: Int) {
   }
 }
 
-// TEST2-LABEL: Processing loops in {{.*}}count_unicodeScalars{{.*}}
-// TEST2: Hoist and Sink pairs attempt
-// TEST2: Hoisted
+// TESTLICM2-LABEL: Processing loops in {{.*}}count_unicodeScalars{{.*}}
+// TESTLICM2: Hoist and Sink pairs attempt
+// TESTLICM2: Hoisted
 
 // TESTSIL-LABEL: sil @$s16licm_exclusivity20count_unicodeScalarsyySS17UnicodeScalarViewVF : $@convention(thin) (@guaranteed String.UnicodeScalarView) -> () {
 // TESTSIL: bb0(%0 : $String.UnicodeScalarView)
@@ -52,34 +52,34 @@ public func count_unicodeScalars(_ s: String.UnicodeScalarView) {
 
 
 public class ClassWithArrs {
-    var N: Int = 0
-    var A: [Int]
-    var B: [Int]
+  var N: Int = 0
+  var A: [Int]
+  var B: [Int]
 
-    init(N: Int) {
-        self.N = N
+  init(N: Int) {
+    self.N = N
 
-        A = [Int](repeating: 0, count: N)
-        B = [Int](repeating: 0, count: N)
-    }
+    A = [Int](repeating: 0, count: N)
+    B = [Int](repeating: 0, count: N)
+  }
 
-// TEST3-LABEL: Processing loops in {{.*}}ClassWithArrsC7readArr{{.*}}
-// TEST3: Hoist and Sink pairs attempt
-// TEST3: Hoisted
-// TEST3: Successfully hosited and sank pair
-// TEST3: Hoisted
-// TEST3: Successfully hosited and sank pair
-// TESTSIL2-LABEL: sil @$s16licm_exclusivity13ClassWithArrsC7readArryyF : $@convention(method) (@guaranteed ClassWithArrs) -> () {
-// TESTSIL2: [[R1:%.*]] = ref_element_addr %0 : $ClassWithArrs, #ClassWithArrs.A
-// TESTSIL2: [[R2:%.*]] = ref_element_addr %0 : $ClassWithArrs, #ClassWithArrs.B
-// TESTSIL2:  begin_access [read] [static] [no_nested_conflict] [[R1]]
-// TESTSIL2:  begin_access [read] [static] [no_nested_conflict] [[R2]]
-    public func readArr() {
-        for i in 0..<self.N {
-            for j in 0..<i {
-				let _ = A[j]
-				let _ = B[j]
-			}
-        }
+// TESTLICMWMO-LABEL: Processing loops in {{.*}}ClassWithArrsC7readArr{{.*}}
+// TESTLICMWMO: Hoist and Sink pairs attempt
+// TESTLICMWMO: Hoisted
+// TESTLICMWMO: Successfully hoisted and sank pair
+// TESTLICMWMO: Hoisted
+// TESTLICMWMO: Successfully hoisted and sank pair
+// TESTSILWMO-LABEL: sil @$s16licm_exclusivity13ClassWithArrsC7readArryyF : $@convention(method) (@guaranteed ClassWithArrs) -> () {
+// TESTSILWMO: [[R1:%.*]] = ref_element_addr %0 : $ClassWithArrs, #ClassWithArrs.A
+// TESTSILWMO: [[R2:%.*]] = ref_element_addr %0 : $ClassWithArrs, #ClassWithArrs.B
+// TESTSILWMO:  begin_access [read] [static] [no_nested_conflict] [[R1]]
+// TESTSILWMO:  begin_access [read] [static] [no_nested_conflict] [[R2]]
+  public func readArr() {
+    for i in 0..<self.N {
+      for j in 0..<i {
+	let _ = A[j]
+	let _ = B[j]
+      }
     }
+  }
 }