[Exclusivity] Teach MemAccessUtils about Projection Paths

Consider a class ‘C’ with distinct fields ‘A’ and ‘B’

And consider we are accessing C.A and C.B inside a loop

LICM well not hoist the exclusivity checking outside of the loop because isDistinctFrom(C.A, C.B) returns false.

This is because the helper function bails if isUniquelyIdentified returns false (which is the case in class kinds)

Same with all other potential access enforcement optimizations.

This PR resolves that

Author: Joe Shajrawi

benchmark/single-source/ExistentialPerformance.swift

@@ -13,6 +13,7 @@
 import TestsUtils
 
 public let ExistentialPerformance = [
+  BenchmarkInfo(name: "DistinctClassFieldAccesses", runFunction: run_DistinctClassFieldAccesses, tags: [.unstable, .api, .Array]),
   BenchmarkInfo(name: "ExistentialTestArrayConditionalShift_ClassValueBuffer1", runFunction: run_ExistentialTestArrayConditionalShift_ClassValueBuffer1, tags: [.unstable, .api, .Array]),
   BenchmarkInfo(name: "ExistentialTestArrayConditionalShift_ClassValueBuffer2", runFunction: run_ExistentialTestArrayConditionalShift_ClassValueBuffer2, tags: [.unstable, .api, .Array]),
   BenchmarkInfo(name: "ExistentialTestArrayConditionalShift_ClassValueBuffer3", runFunction: run_ExistentialTestArrayConditionalShift_ClassValueBuffer3, tags: [.unstable, .api, .Array]),
@@ -114,6 +115,43 @@ public let ExistentialPerformance = [
   BenchmarkInfo(name: "ExistentialTestTwoMethodCalls_IntValueBuffer4", runFunction: run_ExistentialTestTwoMethodCalls_IntValueBuffer4, tags: [.unstable]),
 ]
 
+class ClassWithArrs {
+    var N: Int = 0
+    var A: [Int]
+    var B: [Int]
+
+    init(N: Int) {
+        self.N = N
+
+        A = [Int](repeating: 0, count: N)
+        B = [Int](repeating: 0, count: N)
+    }
+
+    func readArr() {
+        for i in 0..<self.N {
+            for j in 0..<i {
+				let _ = A[j]
+				let _ = B[j]
+			}
+        }
+    }
+
+    func writeArr() {
+		for i in 0..<self.N {
+			A[i] = i
+			B[i] = i
+		}
+    }
+}
+
+public func run_DistinctClassFieldAccesses(_ N: Int) {
+    let workload = ClassWithArrs(N: 100)
+    for _ in 1...N {
+        workload.writeArr()
+        workload.readArr()
+    }
+}
+
 protocol Existential {
   init()
   func doIt() -> Bool

include/swift/SIL/MemAccessUtils.h

@@ -29,27 +29,44 @@ inline bool accessKindMayConflict(SILAccessKind a, SILAccessKind b) {
 }
 
 /// Represents the identity of a stored class property as a combination
-/// of a base and a single projection. Eventually the goal is to make this
-/// more precise and consider, casts, etc.
+/// 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
 class ObjectProjection {
   SILValue object;
+  const RefElementAddrInst *REA;
   Projection proj;
 
 public:
-  ObjectProjection(SILValue object, const Projection &proj)
-      : object(object), proj(proj) {
-    assert(object->getType().isObject());
-  }
+  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; }
 
   SILValue getObject() const { return object; }
+
   const Projection &getProjection() const { return proj; }
 
+  const ProjectionPath getProjectionPath() const {
+    return ProjectionPath::getProjectionPath(stripBorrow(REA->getOperand()),
+                                             REA)
+        .getValue();
+  }
+
   bool operator==(const ObjectProjection &other) const {
-    return object == other.object && proj == other.proj;
+    return getObject() == other.getObject() &&
+           getProjection() == other.getProjection();
   }
 
   bool operator!=(const ObjectProjection &other) const {
-    return object != other.object || proj != other.proj;
+    return !operator==(other);
   }
 };
 
@@ -168,8 +185,8 @@ class AccessedStorage {
 
   AccessedStorage(SILValue base, Kind kind);
 
-  AccessedStorage(SILValue object, Projection projection)
-      : objProj(object, projection) {
+  AccessedStorage(SILValue object, const RefElementAddrInst *REA)
+      : objProj(object, REA) {
     initKind(Class);
   }
 
@@ -255,8 +272,19 @@ class AccessedStorage {
   }
 
   bool isDistinctFrom(const AccessedStorage &other) const {
-    return isUniquelyIdentified() && other.isUniquelyIdentified()
-           && !hasIdenticalBase(other);
+    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();
+    return projPath.hasNonEmptySymmetricDifference(otherProjPath);
   }
 
   /// Returns the ValueDecl for the underlying storage, if it can be

lib/SIL/MemAccessUtils.cpp

@@ -104,8 +104,7 @@ AccessedStorage::AccessedStorage(SILValue base, Kind kind) {
     // actually refer the same address) because these will be dynamically
     // checked.
     auto *REA = cast<RefElementAddrInst>(base);
-    SILValue Object = stripBorrow(REA->getOperand());
-    objProj = ObjectProjection(Object, Projection(REA));
+    objProj = ObjectProjection(REA);
   }
   }
 }

lib/SILOptimizer/Analysis/AccessedStorageAnalysis.cpp

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

test/SILOptimizer/licm_exclusivity.swift

@@ -1,6 +1,9 @@
 // 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  -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
+
 // REQUIRES: optimized_stdlib,asserts
 
 // TEST1-LABEL: Processing loops in {{.*}}run_ReversedArray{{.*}}
@@ -44,3 +47,37 @@ public func count_unicodeScalars(_ s: String.UnicodeScalarView) {
     count += 1
   }
 }
+
+
+public class ClassWithArrs {
+    var N: Int = 0
+    var A: [Int]
+    var B: [Int]
+
+    init(N: Int) {
+        self.N = 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]
+			}
+        }
+    }
+}