Skip to content

Commit 7c07f12

Browse files
committed
[move-only] Improve logging to make it more readable and add a counter to bisect on variables that we are checking.
These are only on in NDEBUG mode. It just makes it easier to quickly triage which variable is causing a checking problem.
1 parent 7a02b26 commit 7c07f12

File tree

1 file changed

+82
-18
lines changed

1 file changed

+82
-18
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 82 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3121,6 +3121,12 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
31213121
// [copy] + begin_borrow for further processing. This just eliminates a case
31223122
// that the checker doesn't need to know about.
31233123
{
3124+
LLVM_DEBUG(llvm::dbgs()
3125+
<< "===>>> Started CopiedLoadBorrowEliminationVisitor!\n");
3126+
SWIFT_DEFER {
3127+
LLVM_DEBUG(llvm::dbgs()
3128+
<< "===<<< Finished CopiedLoadBorrowEliminationVisitor!\n");
3129+
};
31243130
CopiedLoadBorrowEliminationState state(markedAddress->getFunction());
31253131
CopiedLoadBorrowEliminationVisitor copiedLoadBorrowEliminator(state);
31263132
if (AddressUseKind::Unknown ==
@@ -3142,12 +3148,20 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
31423148
// SILGen will treat y as a separate rvalue from x and will create a temporary
31433149
// allocation. In contrast if we have a var, we treat x like an lvalue and
31443150
// just create GEPs appropriately.
3145-
if (eliminateTemporaryAllocationsFromLet(markedAddress)) {
3146-
LLVM_DEBUG(
3147-
llvm::dbgs()
3148-
<< "Succeeded in eliminating temporary allocations! Fn after:\n";
3149-
markedAddress->getFunction()->dump());
3150-
changed = true;
3151+
{
3152+
LLVM_DEBUG(llvm::dbgs() << "===>>> Trying to eliminate temporary "
3153+
"allocations if we have a let!\n");
3154+
SWIFT_DEFER {
3155+
LLVM_DEBUG(llvm::dbgs() << "===<<< Completed eliminate temporary "
3156+
"allocations if we have a let!\n");
3157+
};
3158+
if (eliminateTemporaryAllocationsFromLet(markedAddress)) {
3159+
LLVM_DEBUG(
3160+
llvm::dbgs()
3161+
<< "Succeeded in eliminating temporary allocations! Fn after:\n";
3162+
markedAddress->getFunction()->dump());
3163+
changed = true;
3164+
}
31513165
}
31523166

31533167
// Then gather all uses of our address by walking from def->uses. We use this
@@ -3156,10 +3170,21 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
31563170
GatherUsesVisitor visitor(*this, addressUseState, markedAddress,
31573171
diagnosticEmitter);
31583172
SWIFT_DEFER { visitor.clear(); };
3159-
visitor.reset(markedAddress);
3160-
if (AddressUseKind::Unknown == std::move(visitor).walk(markedAddress)) {
3161-
LLVM_DEBUG(llvm::dbgs() << "Failed access path visit: " << *markedAddress);
3162-
return false;
3173+
3174+
{
3175+
LLVM_DEBUG(llvm::dbgs()
3176+
<< "===>>> Beginning main use gathering visitor!\n");
3177+
SWIFT_DEFER {
3178+
LLVM_DEBUG(llvm::dbgs()
3179+
<< "===<<< Completed main use gathering visitor!\n");
3180+
};
3181+
3182+
visitor.reset(markedAddress);
3183+
if (AddressUseKind::Unknown == std::move(visitor).walk(markedAddress)) {
3184+
LLVM_DEBUG(llvm::dbgs()
3185+
<< "Failed access path visit: " << *markedAddress);
3186+
return false;
3187+
}
31633188
}
31643189

31653190
// If we found a load [copy] or copy_addr that requires multiple copies or an
@@ -3187,18 +3212,27 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
31873212
//===---
31883213
// Liveness Checking
31893214
//
3215+
31903216
SmallVector<SILBasicBlock *, 32> discoveredBlocks;
31913217
FieldSensitiveMultiDefPrunedLiveRange liveness(fn, markedAddress,
31923218
&discoveredBlocks);
3193-
addressUseState.initializeLiveness(liveness);
31943219

3195-
// Then compute the takes that are within the cumulative boundary of
3196-
// liveness that we have computed. If we find any, they are the errors ones.
3197-
GlobalLivenessChecker emitter(addressUseState, diagnosticEmitter, liveness);
3220+
{
3221+
LLVM_DEBUG(llvm::dbgs() << "===>>> Performing liveness checking!\n");
3222+
SWIFT_DEFER {
3223+
LLVM_DEBUG(llvm::dbgs() << "===<<< Completed liveness checking!\n");
3224+
};
31983225

3199-
// If we had any errors, we do not want to modify the SIL... just bail.
3200-
if (emitter.compute()) {
3201-
return true;
3226+
addressUseState.initializeLiveness(liveness);
3227+
3228+
// Then compute the takes that are within the cumulative boundary of
3229+
// liveness that we have computed. If we find any, they are the errors ones.
3230+
GlobalLivenessChecker emitter(addressUseState, diagnosticEmitter, liveness);
3231+
3232+
// If we had any errors, we do not want to modify the SIL... just bail.
3233+
if (emitter.compute()) {
3234+
return true;
3235+
}
32023236
}
32033237

32043238
//===
@@ -3242,15 +3276,40 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
32423276
// MARK: Top Level Entrypoint
32433277
//===----------------------------------------------------------------------===//
32443278

3279+
#ifndef NDEBUG
3280+
static llvm::cl::opt<uint64_t> NumTopLevelToProcess(
3281+
"sil-move-only-address-checker-num-top-level-to-process",
3282+
llvm::cl::desc("Allows for bisecting on move introducer that causes an "
3283+
"error. Only meant for debugging!"),
3284+
llvm::cl::init(UINT64_MAX));
3285+
#endif
3286+
3287+
static llvm::cl::opt<bool> DumpSILBeforeRemovingMarkMustCheck(
3288+
"sil-move-only-address-checker-dump-before-removing-mark-must-check",
3289+
llvm::cl::desc("When bisecting it is useful to dump the SIL before the "
3290+
"rest of the checker removes mark_must_check. This lets one "
3291+
"grab the SIL of a bad variable after all of the rest have "
3292+
"been processed to work with further in sil-opt."),
3293+
llvm::cl::init(false));
3294+
32453295
bool MoveOnlyAddressChecker::check(
32463296
SmallSetVector<MarkMustCheckInst *, 32> &moveIntroducersToProcess) {
32473297
assert(moveIntroducersToProcess.size() &&
32483298
"Must have checks to process to call this function");
32493299
MoveOnlyAddressCheckerPImpl pimpl(fn, diagnosticEmitter, domTree, poa,
32503300
allocator);
32513301

3302+
#ifndef NDEBUG
3303+
static uint64_t numProcessed = 0;
3304+
#endif
32523305
for (auto *markedValue : moveIntroducersToProcess) {
3253-
LLVM_DEBUG(llvm::dbgs() << "Visiting: " << *markedValue);
3306+
#ifndef NDEBUG
3307+
++numProcessed;
3308+
if (NumTopLevelToProcess <= numProcessed)
3309+
break;
3310+
#endif
3311+
LLVM_DEBUG(llvm::dbgs()
3312+
<< "======>>> Visiting top level: " << *markedValue);
32543313

32553314
// Perform our address check.
32563315
unsigned diagnosticEmittedByEarlierPassCount =
@@ -3271,5 +3330,10 @@ bool MoveOnlyAddressChecker::check(
32713330
}
32723331
}
32733332

3333+
if (DumpSILBeforeRemovingMarkMustCheck) {
3334+
LLVM_DEBUG(llvm::dbgs()
3335+
<< "Dumping SIL before removing mark must checks!\n";
3336+
fn->dump());
3337+
}
32743338
return pimpl.changed;
32753339
}

0 commit comments

Comments
 (0)