-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[Exclusivity] Teach MemAccessUtils about Projection Paths #18892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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] | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the point of this benchmark is to check if the access checks are moved out of the loops. IMO, it's not required to have a benchmark for this, because this can be easily checked with a lit test - as you do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I discussed the inclusion of this benchmark with @atrick yesterday, was not sure if we should include it or not, decided to add it to the unstable / exclusivity benchmark package - which we do not run by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice to have a separate data point in the public benchmark suite for some part of a larger benchmark that we care about. On the other hand, if there's nothing unique or interesting about the code aside from the exclusivity check, then it doesn't really add value.