Skip to content

Commit 05808b9

Browse files
swift-ciswift-ci
authored
Merge pull request #24728 from atrick/simplify-accessed-storage-no-rea
2 parents 0de91dc + 0d3c614 commit 05808b9

File tree

6 files changed

+69
-84
lines changed

6 files changed

+69
-84
lines changed

‎include/swift/SIL/MemAccessUtils.h

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -29,43 +29,27 @@ inline bool accessKindMayConflict(SILAccessKind a, SILAccessKind b) {
2929
}
3030

3131
/// Represents the identity of a stored class property as a combination
32-
/// of a base, projection and projection path.
33-
/// we pre-compute the base and projection path, even though we can
34-
/// lazily do so, because it is more expensive otherwise
35-
/// We lazily compute the projection path,
36-
/// In the rare occasions we need it, because of Its destructor and
37-
/// its non-trivial copy constructor
32+
/// of a base and projection.
3833
class ObjectProjection {
3934
SILValue object;
40-
const RefElementAddrInst *REA;
4135
Projection proj;
4236

4337
public:
44-
ObjectProjection(const RefElementAddrInst *REA)
45-
: object(stripBorrow(REA->getOperand())), REA(REA),
46-
proj(Projection(REA)) {}
47-
48-
ObjectProjection(SILValue object, const RefElementAddrInst *REA)
49-
: object(object), REA(REA), proj(Projection(REA)) {}
50-
51-
const RefElementAddrInst *getInstr() const { return REA; }
38+
ObjectProjection(SILValue object, const Projection &projection)
39+
: object(object), proj(projection) {
40+
assert(object->getType().isObject());
41+
}
5242

5343
SILValue getObject() const { return object; }
5444

5545
const Projection &getProjection() const { return proj; }
5646

57-
const Optional<ProjectionPath> getProjectionPath() const {
58-
return ProjectionPath::getProjectionPath(stripBorrow(REA->getOperand()),
59-
REA);
60-
}
61-
6247
bool operator==(const ObjectProjection &other) const {
63-
return getObject() == other.getObject() &&
64-
getProjection() == other.getProjection();
48+
return object == other.object && proj == other.proj;
6549
}
6650

6751
bool operator!=(const ObjectProjection &other) const {
68-
return !operator==(other);
52+
return object != other.object || proj != other.proj;
6953
}
7054
};
7155

@@ -189,8 +173,8 @@ class AccessedStorage {
189173

190174
AccessedStorage(SILValue base, Kind kind);
191175

192-
AccessedStorage(SILValue object, const RefElementAddrInst *REA)
193-
: objProj(object, REA) {
176+
AccessedStorage(SILValue object, Projection projection)
177+
: objProj(object, projection) {
194178
initKind(Class);
195179
}
196180

@@ -295,20 +279,20 @@ class AccessedStorage {
295279
if (isUniquelyIdentified() && other.isUniquelyIdentified()) {
296280
return !hasIdenticalBase(other);
297281
}
298-
if (getKind() != Class || other.getKind() != Class) {
299-
return false;
300-
}
301-
if (getObjectProjection().getProjection() ==
302-
other.getObjectProjection().getProjection()) {
303-
return false;
304-
}
305-
auto projPath = getObjectProjection().getProjectionPath();
306-
auto otherProjPath = other.getObjectProjection().getProjectionPath();
307-
if (!projPath.hasValue() || !otherProjPath.hasValue()) {
282+
if (getKind() != Class || other.getKind() != Class)
283+
// At least one side is an Argument or Yield, or is unidentified.
308284
return false;
285+
286+
// Classes are not uniquely identified by their base. However, if the
287+
// underling objects have identical types and distinct property indices then
288+
// they are distinct storage locations.
289+
auto &proj = getObjectProjection();
290+
auto &otherProj = other.getObjectProjection();
291+
if (proj.getObject()->getType() == otherProj.getObject()->getType()
292+
&& proj.getProjection() != otherProj.getProjection()) {
293+
return true;
309294
}
310-
return projPath.getValue().hasNonEmptySymmetricDifference(
311-
otherProjPath.getValue());
295+
return false;
312296
}
313297

314298
/// Returns the ValueDecl for the underlying storage, if it can be

‎lib/SIL/MemAccessUtils.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,12 @@ AccessedStorage::AccessedStorage(SILValue base, Kind kind) {
101101
case Class: {
102102
// Do a best-effort to find the identity of the object being projected
103103
// from. It is OK to be unsound here (i.e. miss when two ref_element_addrs
104-
// actually refer the same address) because these will be dynamically
105-
// checked.
104+
// actually refer the same address) because these addresses will be
105+
// dynamically checked, and static analysis will be sufficiently
106+
// conservative given that classes are not "uniquely identified".
106107
auto *REA = cast<RefElementAddrInst>(base);
107-
objProj = ObjectProjection(REA);
108+
SILValue Object = stripBorrow(REA->getOperand());
109+
objProj = ObjectProjection(Object, Projection(REA));
108110
}
109111
}
110112
}

‎lib/SILOptimizer/Analysis/AccessedStorageAnalysis.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,9 @@ transformCalleeStorage(const StorageAccessInfo &storage,
238238
if (auto *arg = dyn_cast<SILFunctionArgument>(obj)) {
239239
SILValue argVal = getCallerArg(fullApply, arg->getIndex());
240240
if (argVal) {
241-
auto *instr = storage.getObjectProjection().getInstr();
241+
auto &proj = storage.getObjectProjection().getProjection();
242242
// Remap the argument source value and inherit the old storage info.
243-
return StorageAccessInfo(AccessedStorage(argVal, instr), storage);
243+
return StorageAccessInfo(AccessedStorage(argVal, proj), storage);
244244
}
245245
}
246246
// Otherwise, continue to reference the value in the callee because we don't

‎lib/SILOptimizer/LoopTransforms/LICM.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ hoistSpecialInstruction(std::unique_ptr<LoopNestSummary> &LoopSummary,
347347
llvm_unreachable("LICM: Could not perform must-sink instruction");
348348
}
349349
}
350-
LLVM_DEBUG(llvm::errs() << " Successfully hosited and sank pair\n");
350+
LLVM_DEBUG(llvm::errs() << " Successfully hoisted and sank pair\n");
351351
} else {
352352
LLVM_DEBUG(llvm::dbgs() << "Hoisted RefElementAddr "
353353
<< *static_cast<RefElementAddrInst *>(Inst));

‎test/SILOptimizer/access_enforcement_opts.sil

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,18 +1352,17 @@ class RefElemClass {
13521352
init()
13531353
}
13541354

1355-
// Merge access overlapping scopes. Scope nesting does not need to be
1356-
// preserved unless the nested accesses are to identical storage.
1355+
// Merge access overlapping scopes.
13571356
//
13581357
// CHECK-LABEL: sil @ref_elem_c : $@convention(thin) (RefElemClass) -> () {
13591358
// CHECK: [[GLOBAL:%.*]] = global_addr @globalX : $*X
1360-
// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
1359+
// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
13611360
// CHECK-NEXT: load [[BEGIN]] : $*X
13621361
// CHECK-NEXT: end_access [[BEGIN]] : $*X
13631362
// CHECK-NEXT: [[REFX:%.*]] = ref_element_addr %0 : $RefElemClass, #RefElemClass.x
13641363
// CHECK-NEXT: [[REFY:%.*]] = ref_element_addr %0 : $RefElemClass, #RefElemClass.y
13651364
// CHECK-NEXT: [[BEGINX:%.*]] = begin_access [modify] [dynamic] [[REFX]] : $*BitfieldOne
1366-
// CHECK: [[BEGINY:%.*]] = begin_access [modify] [dynamic] [[REFY]] : $*Int32
1365+
// CHECK: [[BEGINY:%.*]] = begin_access [modify] [dynamic] [no_nested_conflict] [[REFY]] : $*Int32
13671366
// CHECK: end_access [[BEGINX]] : $*BitfieldOne
13681367
// CHECK-NEXT: end_access [[BEGINY]] : $*Int32
13691368
// CHECK-LABEL: } // end sil function 'ref_elem_c'

‎test/SILOptimizer/licm_exclusivity.swift

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1-
// 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
2-
// 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
1+
// 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
2+
// 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
33
// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil -primary-file %s | %FileCheck %s --check-prefix=TESTSIL
4-
// 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
5-
// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil -whole-module-optimization %s | %FileCheck %s --check-prefix=TESTSIL2
4+
// 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
5+
// RUN: %target-swift-frontend -O -enforce-exclusivity=checked -emit-sil -whole-module-optimization %s | %FileCheck %s --check-prefix=TESTSILWMO
66

77
// REQUIRES: optimized_stdlib,asserts
88
// REQUIRES: PTRSIZE=64
99

10-
// TEST1-LABEL: Processing loops in {{.*}}run_ReversedArray{{.*}}
11-
// TEST1: Hoist and Sink pairs attempt
12-
// TEST1: Hoisted
13-
// TEST1: Successfully hosited and sank pair
10+
// TESTLICM-LABEL: Processing loops in {{.*}}run_ReversedArray{{.*}}
11+
// TESTLICM: Hoist and Sink pairs attempt
12+
// TESTLICM: Hoisted
13+
// TESTLICM: Successfully hoisted and sank pair
1414

1515
// TESTSIL-LABEL: sil hidden @$s16licm_exclusivity17run_ReversedArrayyySiF : $@convention(thin) (Int) -> () {
1616
// TESTSIL: bb
@@ -32,9 +32,9 @@ func run_ReversedArray(_ N: Int) {
3232
}
3333
}
3434

35-
// TEST2-LABEL: Processing loops in {{.*}}count_unicodeScalars{{.*}}
36-
// TEST2: Hoist and Sink pairs attempt
37-
// TEST2: Hoisted
35+
// TESTLICM2-LABEL: Processing loops in {{.*}}count_unicodeScalars{{.*}}
36+
// TESTLICM2: Hoist and Sink pairs attempt
37+
// TESTLICM2: Hoisted
3838

3939
// TESTSIL-LABEL: sil @$s16licm_exclusivity20count_unicodeScalarsyySS17UnicodeScalarViewVF : $@convention(thin) (@guaranteed String.UnicodeScalarView) -> () {
4040
// TESTSIL: bb0(%0 : $String.UnicodeScalarView)
@@ -52,34 +52,34 @@ public func count_unicodeScalars(_ s: String.UnicodeScalarView) {
5252

5353

5454
public class ClassWithArrs {
55-
var N: Int = 0
56-
var A: [Int]
57-
var B: [Int]
55+
var N: Int = 0
56+
var A: [Int]
57+
var B: [Int]
5858

59-
init(N: Int) {
60-
self.N = N
59+
init(N: Int) {
60+
self.N = N
6161

62-
A = [Int](repeating: 0, count: N)
63-
B = [Int](repeating: 0, count: N)
64-
}
62+
A = [Int](repeating: 0, count: N)
63+
B = [Int](repeating: 0, count: N)
64+
}
6565

66-
// TEST3-LABEL: Processing loops in {{.*}}ClassWithArrsC7readArr{{.*}}
67-
// TEST3: Hoist and Sink pairs attempt
68-
// TEST3: Hoisted
69-
// TEST3: Successfully hosited and sank pair
70-
// TEST3: Hoisted
71-
// TEST3: Successfully hosited and sank pair
72-
// TESTSIL2-LABEL: sil @$s16licm_exclusivity13ClassWithArrsC7readArryyF : $@convention(method) (@guaranteed ClassWithArrs) -> () {
73-
// TESTSIL2: [[R1:%.*]] = ref_element_addr %0 : $ClassWithArrs, #ClassWithArrs.A
74-
// TESTSIL2: [[R2:%.*]] = ref_element_addr %0 : $ClassWithArrs, #ClassWithArrs.B
75-
// TESTSIL2: begin_access [read] [static] [no_nested_conflict] [[R1]]
76-
// TESTSIL2: begin_access [read] [static] [no_nested_conflict] [[R2]]
77-
public func readArr() {
78-
for i in 0..<self.N {
79-
for j in 0..<i {
80-
let _ = A[j]
81-
let _ = B[j]
82-
}
83-
}
66+
// TESTLICMWMO-LABEL: Processing loops in {{.*}}ClassWithArrsC7readArr{{.*}}
67+
// TESTLICMWMO: Hoist and Sink pairs attempt
68+
// TESTLICMWMO: Hoisted
69+
// TESTLICMWMO: Successfully hoisted and sank pair
70+
// TESTLICMWMO: Hoisted
71+
// TESTLICMWMO: Successfully hoisted and sank pair
72+
// TESTSILWMO-LABEL: sil @$s16licm_exclusivity13ClassWithArrsC7readArryyF : $@convention(method) (@guaranteed ClassWithArrs) -> () {
73+
// TESTSILWMO: [[R1:%.*]] = ref_element_addr %0 : $ClassWithArrs, #ClassWithArrs.A
74+
// TESTSILWMO: [[R2:%.*]] = ref_element_addr %0 : $ClassWithArrs, #ClassWithArrs.B
75+
// TESTSILWMO: begin_access [read] [static] [no_nested_conflict] [[R1]]
76+
// TESTSILWMO: begin_access [read] [static] [no_nested_conflict] [[R2]]
77+
public func readArr() {
78+
for i in 0..<self.N {
79+
for j in 0..<i {
80+
let _ = A[j]
81+
let _ = B[j]
82+
}
8483
}
84+
}
8585
}

0 commit comments

Comments
 (0)