Skip to content

Commit 5e31124

Browse files
committed
[pmo] Update the memory use collector for ownership.
This is technically a NFC commit. The changes are: 1. When we scalarize loads/stores, we need to not just use unqualified loads/stores. Instead we need to use the createTrivial{Load,Store}Or APIs. In OSSA mode, this will propagate through the original ownership qualifier if the sub-type is non-trivial, but if the sub-type is non-trivial will change the qualifier to trivial. Today when the pass runs without ownership nothing is changed since I am passing in the "supports unqualified" flag to the createTrivial{Load,Store}Or API so that we just create an unqualified memop if we are passed in an unqualified memop. Once we fully move pmo to ownership, this flag will be removed and we will assert. 2. The container walker is taught about copy_value, destroy_value. Specifically, we teach the walker how to recursively look through copy_values during the walk and to treat a destroy_value of the box like a strong_release, release_value. Since destroy_value, copy_value only exist in [ossa] today, this is also NFC.
1 parent f348fb1 commit 5e31124

File tree

1 file changed

+21
-8
lines changed

1 file changed

+21
-8
lines changed

lib/SILOptimizer/Mandatory/PMOMemoryUseCollector.cpp

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,9 @@ static SILValue scalarizeLoad(LoadInst *LI,
7878
SmallVector<SILValue, 4> ElementTmps;
7979

8080
for (unsigned i = 0, e = ElementAddrs.size(); i != e; ++i) {
81-
auto *SubLI = B.createLoad(LI->getLoc(), ElementAddrs[i],
82-
LoadOwnershipQualifier::Unqualified);
81+
auto *SubLI = B.createTrivialLoadOr(LI->getLoc(), ElementAddrs[i],
82+
LI->getOwnershipQualifier(),
83+
true /*supports unqualified*/);
8384
ElementTmps.push_back(SubLI);
8485
}
8586

@@ -118,7 +119,7 @@ class ElementUseCollector {
118119

119120
private:
120121
LLVM_NODISCARD bool collectUses(SILValue Pointer);
121-
LLVM_NODISCARD bool collectContainerUses(AllocBoxInst *ABI);
122+
LLVM_NODISCARD bool collectContainerUses(SILValue boxValue);
122123
};
123124
} // end anonymous namespace
124125

@@ -130,8 +131,10 @@ bool ElementUseCollector::collectFrom() {
130131
return collectUses(TheMemory.getAddress());
131132
}
132133

133-
bool ElementUseCollector::collectContainerUses(AllocBoxInst *abi) {
134-
for (auto *ui : abi->getUses()) {
134+
bool ElementUseCollector::collectContainerUses(SILValue boxValue) {
135+
assert(isa<AllocBoxInst>(boxValue) || isa<CopyValueInst>(boxValue));
136+
137+
for (auto *ui : boxValue->getUses()) {
135138
auto *user = ui->getUser();
136139

137140
// dealloc_box deallocated a box containing uninitialized memory. This can
@@ -143,6 +146,14 @@ bool ElementUseCollector::collectContainerUses(AllocBoxInst *abi) {
143146
if (isa<StrongRetainInst>(user) || isa<RetainValueInst>(user))
144147
continue;
145148

149+
// Like retaining, copies do not effect the underlying value. We do need to
150+
// recursively visit the copies users though.
151+
if (auto *cvi = dyn_cast<CopyValueInst>(user)) {
152+
if (!collectContainerUses(cvi))
153+
return false;
154+
continue;
155+
}
156+
146157
// Since we are trying to promote loads/stores, any releases of the box are
147158
// not considered uses of the underlying value due to:
148159
//
@@ -162,7 +173,8 @@ bool ElementUseCollector::collectContainerUses(AllocBoxInst *abi) {
162173
// FIXME: Since we do not support promoting strong_release or release_value
163174
// today this will cause the underlying allocation to never be
164175
// eliminated. That should be implemented and fixed.
165-
if (isa<StrongReleaseInst>(user) || isa<ReleaseValueInst>(user)) {
176+
if (isa<StrongReleaseInst>(user) || isa<ReleaseValueInst>(user) ||
177+
isa<DestroyValueInst>(user)) {
166178
Releases.push_back(user);
167179
continue;
168180
}
@@ -433,8 +445,9 @@ bool ElementUseCollector::collectUses(SILValue Pointer) {
433445
getScalarizedElements(SI->getOperand(0), ElementTmps, SI->getLoc(), B);
434446

435447
for (unsigned i = 0, e = ElementAddrs.size(); i != e; ++i)
436-
B.createStore(SI->getLoc(), ElementTmps[i], ElementAddrs[i],
437-
StoreOwnershipQualifier::Unqualified);
448+
B.createTrivialStoreOr(SI->getLoc(), ElementTmps[i], ElementAddrs[i],
449+
SI->getOwnershipQualifier(),
450+
true /*supports unqualified*/);
438451
SI->eraseFromParent();
439452
continue;
440453
}

0 commit comments

Comments
 (0)