Merge pull request #18892 from shajrawi/class_paths

[Exclusivity] Teach MemAccessUtils about Projection Paths

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]
+			}
+        }
+    }
+}