Skip to content

Commit adeae2f

Browse files
committed
[reference-bindings] Inout Binding ensure that we handle fields correctly
I also: 1. Added some basic SILGen/SILOptimizer/Interpreter tests. 2. Added some logging and cleaned up the pass a little bit. rdar://112033023
1 parent 8f64bce commit adeae2f

File tree

4 files changed

+331
-85
lines changed

4 files changed

+331
-85
lines changed

lib/SILOptimizer/Mandatory/ReferenceBindingTransform.cpp

Lines changed: 174 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,30 @@ static void diagnose(SILInstruction *inst, Diag<T...> diag, U &&...args) {
5656
context.Diags.diagnose(loc.getSourceLoc(), diag, std::forward<U>(args)...);
5757
}
5858

59+
//===----------------------------------------------------------------------===//
60+
// MARK: Utilities
61+
//===----------------------------------------------------------------------===//
62+
63+
namespace {
64+
65+
struct RAIILLVMDebug {
66+
StringRef str;
67+
68+
RAIILLVMDebug(StringRef str) : str(str) {
69+
LLVM_DEBUG(llvm::dbgs() << "===>>> Starting " << str << '\n');
70+
}
71+
72+
RAIILLVMDebug(StringRef str, SILInstruction *u) : str(str) {
73+
LLVM_DEBUG(llvm::dbgs() << "===>>> Starting " << str << ":" << *u);
74+
}
75+
76+
~RAIILLVMDebug() {
77+
LLVM_DEBUG(llvm::dbgs() << "===<<< Completed " << str << '\n');
78+
}
79+
};
80+
81+
} // namespace
82+
5983
//===----------------------------------------------------------------------===//
6084
// MARK: Gather Address Uses
6185
//===----------------------------------------------------------------------===//
@@ -87,6 +111,7 @@ struct ValidateAllUsesWithinLiveness : public AccessUseVisitor {
87111
return true;
88112

89113
if (liveness.isWithinBoundary(user)) {
114+
LLVM_DEBUG(llvm::dbgs() << "User in boundary: " << *user);
90115
diagnose(op->getUser(),
91116
diag::sil_referencebinding_src_used_within_inout_scope);
92117
diagnose(markInst, diag::sil_referencebinding_inout_binding_here);
@@ -103,133 +128,195 @@ struct ValidateAllUsesWithinLiveness : public AccessUseVisitor {
103128
// MARK: Transform
104129
//===----------------------------------------------------------------------===//
105130

106-
static bool runTransform(SILFunction *fn) {
107-
bool madeChange = false;
131+
namespace {
108132

109-
StackList<MarkUnresolvedReferenceBindingInst *> targets(fn);
110-
for (auto &block : *fn) {
111-
for (auto &ii : block) {
112-
if (auto *murbi = dyn_cast<MarkUnresolvedReferenceBindingInst>(&ii))
113-
targets.push_back(murbi);
114-
}
133+
struct DiagnosticEmitter {
134+
unsigned numDiagnostics = 0;
135+
136+
void diagnoseUnknownPattern(MarkUnresolvedReferenceBindingInst *mark) {
137+
diagnose(mark, diag::sil_referencebinding_unknown_pattern);
138+
++numDiagnostics;
115139
}
140+
};
116141

117-
bool emittedError = false;
118-
while (!targets.empty()) {
119-
auto *mark = targets.pop_back_val();
120-
SWIFT_DEFER {
121-
mark->replaceAllUsesWith(mark->getOperand());
122-
mark->eraseFromParent();
123-
madeChange = true;
124-
};
125-
126-
// We rely on the following semantics to find our initialization:
127-
//
128-
// 1. SILGen always initializes values completely.
129-
// 2. Boxes always have operations guarded /except/ for their
130-
// initialization.
131-
// 3. We force inout to always be initialized once.
132-
//
133-
// Thus to find our initialization, we need to find a project_box use of our
134-
// target that directly initializes the value.
135-
CopyAddrInst *initInst = nullptr;
136-
for (auto *use : mark->getUses()) {
137-
if (auto *pbi = dyn_cast<ProjectBoxInst>(use->getUser())) {
138-
for (auto *use : pbi->getUses()) {
139-
if (auto *cai = dyn_cast<CopyAddrInst>(use->getUser())) {
140-
if (cai->getDest() == pbi) {
141-
if (initInst || !cai->isInitializationOfDest() ||
142-
cai->isTakeOfSrc()) {
143-
emittedError = true;
144-
diagnose(mark, diag::sil_referencebinding_unknown_pattern);
145-
break;
146-
}
147-
assert(!initInst && "Init twice?!");
148-
assert(!cai->isTakeOfSrc());
149-
initInst = cai;
150-
continue;
151-
}
152-
}
142+
} // namespace
143+
144+
namespace {
145+
146+
class ReferenceBindingProcessor {
147+
MarkUnresolvedReferenceBindingInst *mark;
148+
DiagnosticEmitter &diagnosticEmitter;
149+
150+
public:
151+
ReferenceBindingProcessor(MarkUnresolvedReferenceBindingInst *mark,
152+
DiagnosticEmitter &diagnosticEmitter)
153+
: mark(mark), diagnosticEmitter(diagnosticEmitter) {}
154+
155+
bool process() &&;
156+
157+
private:
158+
CopyAddrInst *findInit();
159+
};
160+
161+
} // namespace
162+
163+
CopyAddrInst *ReferenceBindingProcessor::findInit() {
164+
RAIILLVMDebug llvmDebug("Find initialization");
165+
166+
// We rely on the following semantics to find our initialization:
167+
//
168+
// 1. SILGen always initializes values completely.
169+
// 2. Boxes always have operations guarded /except/ for their
170+
// initialization.
171+
// 3. We force inout to always be initialized once.
172+
//
173+
// Thus to find our initialization, we need to find a project_box use of our
174+
// target that directly initializes the value.
175+
CopyAddrInst *initInst = nullptr;
176+
for (auto *use : mark->getUses()) {
177+
LLVM_DEBUG(llvm::dbgs() << "Visiting use: " << *use->getUser());
178+
179+
if (auto *pbi = dyn_cast<ProjectBoxInst>(use->getUser())) {
180+
LLVM_DEBUG(llvm::dbgs() << " Found project_box! Visiting pbi uses!\n");
181+
182+
for (auto *pbiUse : pbi->getUses()) {
183+
LLVM_DEBUG(llvm::dbgs() << " Pbi Use: " << *pbiUse->getUser());
184+
auto *cai = dyn_cast<CopyAddrInst>(pbiUse->getUser());
185+
if (!cai || cai->getDest() != pbi) {
186+
LLVM_DEBUG(llvm::dbgs() << " Either not a copy_addr or dest is "
187+
"not the project_box! Skipping!\n");
188+
continue;
153189
}
154190

155-
if (emittedError)
156-
break;
191+
if (initInst || !cai->isInitializationOfDest() || cai->isTakeOfSrc()) {
192+
LLVM_DEBUG(
193+
llvm::dbgs()
194+
<< " Either already found an init inst or is an assign of a "
195+
"dest or a take of src... emitting unknown pattern!\n");
196+
diagnosticEmitter.diagnoseUnknownPattern(mark);
197+
return nullptr;
198+
}
199+
assert(!initInst && "Init twice?!");
200+
assert(!cai->isTakeOfSrc());
201+
initInst = cai;
202+
LLVM_DEBUG(llvm::dbgs()
203+
<< " Found our init! Checking for other inits!\n");
157204
}
158205
}
206+
}
207+
LLVM_DEBUG(llvm::dbgs() << "Final Init: " << *initInst);
208+
return initInst;
209+
}
159210

160-
if (emittedError)
161-
continue;
162-
assert(initInst && "Failed to find single initialization");
211+
bool ReferenceBindingProcessor::process() && {
212+
// Find a single initialization of our inout parameter. See helper function
213+
// for the information about the invariants we rely upon.
214+
auto *initInst = findInit();
215+
if (!initInst)
216+
return false;
217+
218+
// Ok, we have our initialization. Now gather our destroys of our value and
219+
// initialize an SSAPrunedLiveness, using our initInst as our def.
220+
auto *fn = mark->getFunction();
221+
222+
SmallVector<SILBasicBlock *, 8> discoveredBlocks;
223+
SSAPrunedLiveness liveness(fn, &discoveredBlocks);
224+
StackList<DestroyValueInst *> destroyValueInst(fn);
225+
{
226+
RAIILLVMDebug llvmDebug("Initializing liveness!");
163227

164-
// Ok, we have our initialization. Now gather our destroys of our value and
165-
// initialize an SSAPrunedLiveness, using our initInst as our def.
166-
SmallVector<SILBasicBlock *, 8> discoveredBlocks;
167-
SSAPrunedLiveness liveness(fn, &discoveredBlocks);
168-
StackList<DestroyValueInst *> destroyValueInst(fn);
169228
liveness.initializeDef(mark);
170229
for (auto *consume : mark->getConsumingUses()) {
171230
// Make sure that the destroy_value is not within the boundary.
172231
auto *dai = dyn_cast<DestroyValueInst>(consume->getUser());
173232
if (!dai) {
174-
emittedError = true;
175-
diagnose(mark, diag::sil_referencebinding_unknown_pattern);
176-
break;
233+
LLVM_DEBUG(llvm::dbgs() << " Found non destroy value consuming use! "
234+
"Emitting unknown pattern: "
235+
<< *dai);
236+
diagnosticEmitter.diagnoseUnknownPattern(mark);
237+
return false;
177238
}
178239

240+
LLVM_DEBUG(llvm::dbgs() << " Found destroy_value use: " << *dai);
179241
liveness.updateForUse(dai, true /*is lifetime ending*/);
180242
destroyValueInst.push_back(dai);
181243
}
182-
if (emittedError)
183-
continue;
244+
}
184245

185-
// Now make sure that our source address doesn't have any uses within the
186-
// lifetime of our box. Or emit an error.
187-
auto accessPathWithBase = AccessPathWithBase::compute(initInst->getSrc());
246+
// Now make sure that our source address doesn't have any uses within the
247+
// lifetime of our box. Or emit an error. Since sema always ensures that we
248+
// have an lvalue, we should always find an access scope.
249+
BeginAccessInst *bai = nullptr;
250+
StackList<SILInstruction *> endAccesses(fn);
251+
{
252+
auto accessPathWithBase =
253+
AccessPathWithBase::computeInScope(initInst->getSrc());
188254
assert(accessPathWithBase.base);
189255

190256
// Then search up for a single begin_access from src to find our
191257
// begin_access we need to fix. We know that it will always be directly on
192258
// the type since we only allow for inout today to be on decl_ref expr. This
193259
// will change in the future. Once we find that begin_access, we need to
194260
// convert it to a modify and expand it.
195-
auto *bai = cast<BeginAccessInst>(initInst->getSrc());
261+
bai = cast<BeginAccessInst>(accessPathWithBase.base);
196262
assert(bai->getAccessKind() == SILAccessKind::Read);
197-
StackList<SILInstruction *> endAccesses(fn);
198263
for (auto *eai : bai->getEndAccesses())
199264
endAccesses.push_back(eai);
200265

201266
{
202267
ValidateAllUsesWithinLiveness initGatherer(liveness, mark, initInst);
203268
if (!visitAccessPathBaseUses(initGatherer, accessPathWithBase, fn)) {
204-
emittedError = true;
205-
diagnose(mark, diag::sil_referencebinding_unknown_pattern);
269+
diagnosticEmitter.diagnoseUnknownPattern(mark);
270+
return false;
206271
}
207-
emittedError |= initGatherer.emittedDiagnostic;
208272
}
209-
if (emittedError)
210-
continue;
273+
}
211274

212-
initInst->setIsTakeOfSrc(IsTake);
213-
bai->setAccessKind(SILAccessKind::Modify);
214-
madeChange = true;
275+
initInst->setIsTakeOfSrc(IsTake);
276+
bai->setAccessKind(SILAccessKind::Modify);
277+
278+
while (!endAccesses.empty())
279+
endAccesses.pop_back_val()->eraseFromParent();
280+
281+
while (!destroyValueInst.empty()) {
282+
auto *consume = destroyValueInst.pop_back_val();
283+
SILBuilderWithScope builder(consume);
284+
auto *pbi = builder.createProjectBox(consume->getLoc(), mark, 0);
285+
builder.createCopyAddr(consume->getLoc(), pbi, initInst->getSrc(), IsTake,
286+
IsInitialization);
287+
builder.createEndAccess(consume->getLoc(), bai, false /*aborted*/);
288+
builder.createDeallocBox(consume->getLoc(), mark);
289+
consume->eraseFromParent();
290+
}
215291

216-
while (!endAccesses.empty())
217-
endAccesses.pop_back_val()->eraseFromParent();
292+
return true;
293+
}
218294

219-
{
220-
while (!destroyValueInst.empty()) {
221-
auto *consume = destroyValueInst.pop_back_val();
222-
SILBuilderWithScope builder(consume);
223-
auto *pbi = builder.createProjectBox(consume->getLoc(), mark, 0);
224-
builder.createCopyAddr(consume->getLoc(), pbi, initInst->getSrc(),
225-
IsTake, IsInitialization);
226-
builder.createEndAccess(consume->getLoc(), bai, false /*aborted*/);
227-
builder.createDeallocBox(consume->getLoc(), mark);
228-
consume->eraseFromParent();
229-
}
295+
static bool runTransform(SILFunction *fn) {
296+
bool madeChange = false;
297+
298+
// First go through and find all of our mark_unresolved_reference_binding.
299+
StackList<MarkUnresolvedReferenceBindingInst *> targets(fn);
300+
for (auto &block : *fn) {
301+
for (auto &ii : block) {
302+
if (auto *murbi = dyn_cast<MarkUnresolvedReferenceBindingInst>(&ii))
303+
targets.push_back(murbi);
230304
}
231305
}
232306

307+
DiagnosticEmitter emitter;
308+
while (!targets.empty()) {
309+
auto *mark = targets.pop_back_val();
310+
311+
LLVM_DEBUG(llvm::dbgs() << "===> Visiting mark: " << *mark);
312+
ReferenceBindingProcessor processor{mark, emitter};
313+
madeChange |= std::move(processor).process();
314+
315+
mark->replaceAllUsesWith(mark->getOperand());
316+
mark->eraseFromParent();
317+
madeChange = true;
318+
}
319+
233320
return madeChange;
234321
}
235322

@@ -247,6 +334,8 @@ struct ReferenceBindingTransformPass : SILFunctionTransform {
247334
if (!fn->getASTContext().LangOpts.hasFeature(Feature::ReferenceBindings))
248335
return;
249336

337+
LLVM_DEBUG(llvm::dbgs()
338+
<< "!!! === RefBindingTransform: " << fn->getName() << '\n');
250339
if (runTransform(fn)) {
251340
invalidateAnalysis(SILAnalysis::Instructions);
252341
}

0 commit comments

Comments
 (0)