Skip to content

Commit c588c65

Browse files
committed
SILVerifier - option to verify with or without linear lifetime check
Add a separate 'verifyOwnership()' entry point so it's possible to check OSSA lifetimes at various points. Move SILGenCleanup into a SILGen pass pipeline. After SILGen, verify incomplete OSSA. After SILGenCleanup, verify ownership.
1 parent 103a6fe commit c588c65

File tree

10 files changed

+115
-37
lines changed

10 files changed

+115
-37
lines changed

include/swift/SIL/SILFunction.h

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,18 +1403,30 @@ class SILFunction
14031403
decl->getLifetimeAnnotation());
14041404
}
14051405

1406-
/// verify - Run the IR verifier to make sure that the SILFunction follows
1406+
/// verify - Run the SIL verifier to make sure that the SILFunction follows
14071407
/// invariants.
1408-
void verify(bool SingleFunction = true) const;
1408+
void verify(bool SingleFunction = true,
1409+
bool isCompleteOSSA = true,
1410+
bool checkLinearLifetime = true) const;
1411+
1412+
/// Run the SIL verifier without assuming OSSA lifetimes end at dead end
1413+
/// blocks.
1414+
void verifyIncompleteOSSA() const {
1415+
verify(/*SingleFunction=*/true, /*completeOSSALifetimes=*/false);
1416+
}
14091417

14101418
/// Verifies the lifetime of memory locations in the function.
14111419
void verifyMemoryLifetime();
14121420

1413-
/// Run the SIL ownership verifier to check for ownership invariant failures.
1421+
/// Run the SIL ownership verifier to check that all values with ownership
1422+
/// have a linear lifetime. Regular OSSA invariants are checked separately in
1423+
/// normal SIL verification.
1424+
///
1425+
/// \p deadEndBlocks is nullptr when OSSA lifetimes are complete.
14141426
///
1415-
/// NOTE: The ownership verifier is always run when performing normal IR
1427+
/// NOTE: The ownership verifier is run when performing normal IR
14161428
/// verification, so this verification can be viewed as a subset of
1417-
/// SILFunction::verify.
1429+
/// SILFunction::verify(checkLinearLifetimes=true).
14181430
void verifyOwnership(DeadEndBlocks *deadEndBlocks) const;
14191431

14201432
/// Verify that all non-cond-br critical edges have been split.

include/swift/SIL/SILModule.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,17 @@ class SILModule {
904904

905905
/// Run the SIL verifier to make sure that all Functions follow
906906
/// invariants.
907-
void verify() const;
907+
void verify(bool isCompleteOSSA = true,
908+
bool checkLinearLifetime = true) const;
909+
910+
/// Run the SIL verifier without assuming OSSA lifetimes end at dead end
911+
/// blocks.
912+
void verifyIncompleteOSSA() const {
913+
verify(/*isCompleteOSSA=*/false);
914+
}
915+
916+
/// Check linear OSSA lifetimes, assuming complete OSSA.
917+
void verifyOwnership() const;
908918

909919
/// Check if there are any leaking instructions.
910920
///

include/swift/SIL/SILValue.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,8 @@ class SILValue {
675675
}
676676

677677
/// Verify that this SILValue and its uses respects ownership invariants.
678+
///
679+
/// \p DEBlocks is nullptr when OSSA lifetimes are complete.
678680
void verifyOwnership(DeadEndBlocks *DEBlocks) const;
679681

680682
SWIFT_DEBUG_DUMP;

include/swift/SILOptimizer/PassManager/PassPipeline.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#define PASSPIPELINE(NAME, DESCRIPTION)
2323
#endif
2424

25+
PASSPIPELINE(SILGen, "SILGen Passes")
2526
PASSPIPELINE(Diagnostic, "Guaranteed Passes")
2627
PASSPIPELINE(LowerHopToActor, "Lower Hop to Actor")
2728
PASSPIPELINE(OwnershipEliminator, "Utility pass to just run the ownership eliminator pass")

lib/SIL/Verifier/LoadBorrowImmutabilityChecker.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ bool GatherWritesVisitor::visitUse(Operand *op, AccessUseType useTy) {
307307
//===----------------------------------------------------------------------===//
308308

309309
LoadBorrowImmutabilityAnalysis::LoadBorrowImmutabilityAnalysis(
310-
DeadEndBlocks &deadEndBlocks, const SILFunction *f)
310+
DeadEndBlocks *deadEndBlocks, const SILFunction *f)
311311
: cache(GatherWrites(f)), deadEndBlocks(deadEndBlocks) {}
312312

313313
// \p address may be an address, pointer, or box type.
@@ -333,7 +333,7 @@ bool LoadBorrowImmutabilityAnalysis::isImmutableInScope(
333333
for (auto *op : *writes) {
334334
auto *write = op->getUser();
335335
// First see if the write is a dead end block. In such a case, just skip it.
336-
if (deadEndBlocks.isDeadEnd(write->getParent())) {
336+
if (deadEndBlocks && deadEndBlocks->isDeadEnd(write->getParent())) {
337337
continue;
338338
}
339339

lib/SIL/Verifier/SILOwnershipVerifier.cpp

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class SILValueOwnershipChecker {
8686

8787
/// A cache of dead-end basic blocks that we use to determine if we can
8888
/// ignore "leaks".
89-
DeadEndBlocks &deadEndBlocks;
89+
DeadEndBlocks *deadEndBlocks = nullptr;
9090

9191
/// The value whose ownership we will check.
9292
SILValue value;
@@ -106,7 +106,8 @@ class SILValueOwnershipChecker {
106106
GuaranteedPhiVerifier &guaranteedPhiVerifier;
107107

108108
public:
109-
SILValueOwnershipChecker(DeadEndBlocks &deadEndBlocks, SILValue value,
109+
/// \p deadEndBlocks is nullptr for complete OSSA lifetimes
110+
SILValueOwnershipChecker(DeadEndBlocks *deadEndBlocks, SILValue value,
110111
LinearLifetimeChecker::ErrorBuilder &errorBuilder,
111112
GuaranteedPhiVerifier &guaranteedPhiVerifier)
112113
: result(), deadEndBlocks(deadEndBlocks), value(value),
@@ -485,7 +486,7 @@ bool SILValueOwnershipChecker::checkFunctionArgWithoutLifetimeEndingUses(
485486
break;
486487
}
487488

488-
if (deadEndBlocks.isDeadEnd(arg->getParent()))
489+
if (deadEndBlocks && deadEndBlocks->isDeadEnd(arg->getParent()))
489490
return true;
490491

491492
return !errorBuilder.handleMalformedSIL([&] {
@@ -503,9 +504,10 @@ bool SILValueOwnershipChecker::checkYieldWithoutLifetimeEndingUses(
503504
case OwnershipKind::None:
504505
return true;
505506
case OwnershipKind::Owned:
506-
if (deadEndBlocks.isDeadEnd(yield->getParent()->getParent()))
507+
if (deadEndBlocks
508+
&& deadEndBlocks->isDeadEnd(yield->getParent()->getParent())) {
507509
return true;
508-
510+
}
509511
return !errorBuilder.handleMalformedSIL([&] {
510512
llvm::errs() << "Owned yield without life ending uses!\n"
511513
<< "Value: " << *yield << '\n';
@@ -572,7 +574,7 @@ bool SILValueOwnershipChecker::checkValueWithoutLifetimeEndingUses(
572574
return true;
573575

574576
if (auto *parentBlock = value->getParentBlock()) {
575-
if (deadEndBlocks.isDeadEnd(parentBlock)) {
577+
if (deadEndBlocks && deadEndBlocks->isDeadEnd(parentBlock)) {
576578
LLVM_DEBUG(llvm::dbgs() << "Ignoring transitively unreachable value "
577579
<< "without users!\n"
578580
<< " Value: " << *value << '\n');
@@ -833,7 +835,7 @@ verifySILValueHelper(const SILFunction *f, SILValue value,
833835
if (!f->hasOwnership() || !f->shouldVerifyOwnership())
834836
return;
835837

836-
SILValueOwnershipChecker(*deadEndBlocks, value, errorBuilder,
838+
SILValueOwnershipChecker(deadEndBlocks, value, errorBuilder,
837839
guaranteedPhiVerifier)
838840
.check();
839841
}
@@ -882,11 +884,32 @@ void SILValue::verifyOwnership(DeadEndBlocks *deadEndBlocks) const {
882884
using BehaviorKind = LinearLifetimeChecker::ErrorBehaviorKind;
883885
LinearLifetimeChecker::ErrorBuilder errorBuilder(
884886
*f, BehaviorKind::PrintMessageAndAssert);
885-
GuaranteedPhiVerifier guaranteedPhiVerifier(f, *deadEndBlocks, errorBuilder);
887+
GuaranteedPhiVerifier guaranteedPhiVerifier(f, deadEndBlocks, errorBuilder);
886888
verifySILValueHelper(f, *this, errorBuilder, deadEndBlocks,
887889
guaranteedPhiVerifier);
888890
}
889891

892+
void SILModule::verifyOwnership() const {
893+
if (DisableOwnershipVerification)
894+
return;
895+
896+
#ifdef NDEBUG
897+
// When compiling without asserts enabled, only verify ownership if
898+
// -sil-verify-all is set.
899+
if (!getOptions().VerifyAll)
900+
return;
901+
#endif
902+
903+
for (const SILFunction &function : *this) {
904+
std::unique_ptr<DeadEndBlocks> deBlocks;
905+
if (!getOptions().OSSACompleteLifetimes) {
906+
deBlocks =
907+
std::make_unique<DeadEndBlocks>(const_cast<SILFunction *>(&function));
908+
}
909+
function.verifyOwnership(deBlocks.get());
910+
}
911+
}
912+
890913
void SILFunction::verifyOwnership(DeadEndBlocks *deadEndBlocks) const {
891914
if (DisableOwnershipVerification)
892915
return;
@@ -919,7 +942,7 @@ void SILFunction::verifyOwnership(DeadEndBlocks *deadEndBlocks) const {
919942
errorBuilder.emplace(*this, BehaviorKind::PrintMessageAndAssert);
920943
}
921944

922-
GuaranteedPhiVerifier guaranteedPhiVerifier(this, *deadEndBlocks,
945+
GuaranteedPhiVerifier guaranteedPhiVerifier(this, deadEndBlocks,
923946
*errorBuilder);
924947
for (auto &block : *this) {
925948
for (auto *arg : block.getArguments()) {

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -702,17 +702,21 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
702702
const SILFunction &F;
703703
SILFunctionConventions fnConv;
704704
Lowering::TypeConverter &TC;
705+
706+
bool SingleFunction = true;
707+
bool checkLinearLifetime = false;
708+
705709
SmallVector<std::pair<StringRef, SILType>, 16> DebugVars;
706710
const SILInstruction *CurInstruction = nullptr;
707711
const SILArgument *CurArgument = nullptr;
708712
std::unique_ptr<DominanceInfo> Dominance;
709713

710714
// Used for dominance checking within a basic block.
711715
llvm::DenseMap<const SILInstruction *, unsigned> InstNumbers;
712-
713-
DeadEndBlocks DEBlocks;
716+
717+
std::unique_ptr<DeadEndBlocks> DEBlocks;
718+
714719
LoadBorrowImmutabilityAnalysis loadBorrowImmutabilityAnalysis;
715-
bool SingleFunction = true;
716720

717721
/// A cache of the isOperandInValueUse check. When we process an operand, we
718722
/// fix this for each of its uses.
@@ -968,13 +972,13 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
968972
SILVerifier(const SILFunction &F, bool SingleFunction = true)
969973
: M(F.getModule().getSwiftModule()), F(F),
970974
fnConv(F.getConventionsInContext()), TC(F.getModule().Types),
975+
SingleFunction(SingleFunction),
971976
Dominance(nullptr),
972-
InstNumbers(numInstsInFunction(F)), DEBlocks(&F),
973-
loadBorrowImmutabilityAnalysis(DEBlocks, &F),
974-
SingleFunction(SingleFunction) {
977+
InstNumbers(numInstsInFunction(F)),
978+
loadBorrowImmutabilityAnalysis(DEBlocks.get(), &F) {
975979
if (F.isExternalDeclaration())
976980
return;
977-
981+
978982
// Check to make sure that all blocks are well formed. If not, the
979983
// SILVerifier object will explode trying to compute dominance info.
980984
unsigned InstIdx = 0;
@@ -1067,7 +1071,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
10671071
void visitSILArgument(SILArgument *arg) {
10681072
CurArgument = arg;
10691073
checkLegalType(arg->getFunction(), arg, nullptr);
1070-
checkValueBaseOwnership(arg);
1074+
if (checkLinearLifetime) {
1075+
checkValueBaseOwnership(arg);
1076+
}
10711077
if (auto *phiArg = dyn_cast<SILPhiArgument>(arg)) {
10721078
if (phiArg->isPhi())
10731079
visitSILPhiArgument(phiArg);
@@ -1107,7 +1113,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
11071113

11081114
for (auto result : I->getResults()) {
11091115
checkLegalType(F, result, I);
1110-
checkValueBaseOwnership(result);
1116+
if (checkLinearLifetime) {
1117+
checkValueBaseOwnership(result);
1118+
}
11111119
}
11121120
}
11131121

@@ -1125,7 +1133,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
11251133
"Once ownership is gone, all values should have none ownership");
11261134
return;
11271135
}
1128-
SILValue(V).verifyOwnership(&DEBlocks);
1136+
SILValue(V).verifyOwnership(DEBlocks.get());
11291137
}
11301138

11311139
void checkSILInstruction(SILInstruction *I) {
@@ -6408,7 +6416,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
64086416
}
64096417
}
64106418

6411-
void verify() {
6419+
void verify(bool isCompleteOSSA, bool checkLinearLifetimes) {
6420+
if (!isCompleteOSSA || !F.getModule().getOptions().OSSACompleteLifetimes) {
6421+
DEBlocks = std::make_unique<DeadEndBlocks>(const_cast<SILFunction *>(&F));
6422+
}
64126423
visitSILFunction(const_cast<SILFunction*>(&F));
64136424
}
64146425
};
@@ -6441,14 +6452,15 @@ static bool verificationEnabled(const SILModule &M) {
64416452

64426453
/// verify - Run the SIL verifier to make sure that the SILFunction follows
64436454
/// invariants.
6444-
void SILFunction::verify(bool SingleFunction) const {
6455+
void SILFunction::verify(bool SingleFunction, bool isCompleteOSSA,
6456+
bool checkLinearLifetime) const {
64456457
if (!verificationEnabled(getModule()))
64466458
return;
64476459

64486460
// Please put all checks in visitSILFunction in SILVerifier, not here. This
64496461
// ensures that the pretty stack trace in the verifier is included with the
64506462
// back trace when the verifier crashes.
6451-
SILVerifier(*this, SingleFunction).verify();
6463+
SILVerifier(*this, SingleFunction).verify(isCompleteOSSA, checkLinearLifetime);
64526464
}
64536465

64546466
void SILFunction::verifyCriticalEdges() const {
@@ -6717,7 +6729,7 @@ void SILGlobalVariable::verify() const {
67176729
}
67186730

67196731
/// Verify the module.
6720-
void SILModule::verify() const {
6732+
void SILModule::verify(bool isCompleteOSSA, bool checkLinearLifetime) const {
67216733
if (!verificationEnabled(*this))
67226734
return;
67236735

@@ -6732,7 +6744,7 @@ void SILModule::verify() const {
67326744
llvm::errs() << "Symbol redefined: " << f.getName() << "!\n";
67336745
assert(false && "triggering standard assertion failure routine");
67346746
}
6735-
f.verify(/*singleFunction*/ false);
6747+
f.verify(/*singleFunction*/ false, isCompleteOSSA, checkLinearLifetime);
67366748
}
67376749

67386750
// Check all globals.

lib/SIL/Verifier/VerifierPrivate.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@ namespace silverifier {
2828

2929
class LoadBorrowImmutabilityAnalysis {
3030
SmallMultiMapCache<AccessPath, Operand *> cache;
31-
DeadEndBlocks &deadEndBlocks;
31+
DeadEndBlocks *deadEndBlocks = nullptr;
3232

3333
public:
34-
LoadBorrowImmutabilityAnalysis(DeadEndBlocks &deadEndBlocks,
34+
/// \p deadEndBlocks should be nullptr to enforce complete OSSA lifetimes.
35+
LoadBorrowImmutabilityAnalysis(DeadEndBlocks *deadEndBlocks,
3536
const SILFunction *f);
3637

3738
/// Returns true if exhaustively lbi is guaranteed to never be invalidated by

lib/SILOptimizer/PassManager/PassPipeline.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ static void addDefiniteInitialization(SILPassPipelinePlan &P) {
115115
// should be in the -Onone pass pipeline and the prepare optimizations pipeline.
116116
static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
117117
P.startPipeline("Mandatory Diagnostic Passes + Enabling Optimization Passes");
118-
P.addSILGenCleanup();
119118
P.addDiagnoseInvalidEscapingCaptures();
120119
P.addDiagnoseStaticExclusivity();
121120
P.addNestedSemanticFunctionCheck();
@@ -235,12 +234,21 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
235234
}
236235

237236
SILPassPipelinePlan
238-
SILPassPipelinePlan::getDiagnosticPassPipeline(const SILOptions &Options) {
237+
SILPassPipelinePlan::getSILGenPassPipeline(const SILOptions &Options) {
239238
SILPassPipelinePlan P(Options);
239+
P.startPipeline("SILGen Passes");
240+
241+
P.addSILGenCleanup();
240242

241243
if (SILViewSILGenCFG) {
242244
addCFGPrinterPipeline(P, "SIL View SILGen CFG");
243245
}
246+
return P;
247+
}
248+
249+
SILPassPipelinePlan
250+
SILPassPipelinePlan::getDiagnosticPassPipeline(const SILOptions &Options) {
251+
SILPassPipelinePlan P(Options);
244252

245253
// If we are asked do debug serialization, instead of running all diagnostic
246254
// passes, just run mandatory inlining with dead transparent function cleanup

lib/SILOptimizer/PassManager/Passes.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
12
//===--- Passes.cpp - Swift Compiler SIL Pass Entrypoints -----------------===//
23
//
34
// This source file is part of the Swift.org open source project
@@ -46,14 +47,22 @@ bool swift::runSILDiagnosticPasses(SILModule &Module) {
4647
auto &opts = Module.getOptions();
4748

4849
// Verify the module, if required.
50+
// OSSA lifetimes are incomplete until the SILGenCleanup pass runs.
4951
if (opts.VerifyAll)
50-
Module.verify();
52+
Module.verifyIncompleteOSSA();
5153

5254
// If we parsed a .sil file that is already in canonical form, don't rerun
5355
// the diagnostic passes.
5456
if (Module.getStage() != SILStage::Raw)
5557
return false;
5658

59+
executePassPipelinePlan(&Module,
60+
SILPassPipelinePlan::getSILGenPassPipeline(opts),
61+
/*isMandatory*/ true);
62+
63+
if (opts.VerifyAll && opts.OSSACompleteLifetimes)
64+
Module.verifyOwnership();
65+
5766
executePassPipelinePlan(&Module,
5867
SILPassPipelinePlan::getDiagnosticPassPipeline(opts),
5968
/*isMandatory*/ true);

0 commit comments

Comments
 (0)