Skip to content

Commit f462d20

Browse files
committed
[ownership-verifier] Rename PrintMessageInsteadOfAssert => IsSILOwnershipVerifierTestingEnabled and use it to disable SILInstruction::verifyOperandOwnership
The method SILInstruction::verifyOperandOwnership() is used in SILBuilder to ensure that as instructions are created, if there are any use failures, the error is caught at the moment the instruction is created. This makes debugging such failures trivial. *NOTE* This does not cause dataflow verification to occur. The problem is that when PrintMessageInsteadOfAssert is enabled, this causes dataflow failures to have their error messages emitted twice, complicating FileCheck testing of the verifier. Thus, this commit disables SILInstruction::verifyOperandOwnership() when PrintMessageInsteadOfAssert is enabled. PrintMessageInsteadOfAssert is renamed to 'IsSILOwnershipVerifierTestingEnabled' in light of its expanded role. rdar://29791263
1 parent e9dd810 commit f462d20

File tree

2 files changed

+30
-15
lines changed

2 files changed

+30
-15
lines changed

lib/SIL/SILOwnershipVerifier.cpp

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,21 @@ using namespace swift;
4242
// prevent release builds from triggering spurious unused variable warnings.
4343
#ifndef NDEBUG
4444

45-
llvm::cl::opt<bool> PrintMessageInsteadOfAssert(
46-
"sil-ownership-verifier-do-not-assert",
47-
llvm::cl::desc("Print out message instead of asserting. "
48-
"Meant for debugging"));
45+
// This is an option to put the SILOwnershipVerifier in testing mode. This
46+
// causes the following:
47+
//
48+
// 1. Instead of printing an error message and aborting, the verifier will print
49+
// the message and continue. This allows for FileCheck testing of the verifier.
50+
//
51+
// 2. SILInstruction::verifyOperandOwnership() is disabled. This is used for
52+
// verification in SILBuilder. This causes errors to be printed twice, once when
53+
// we build the IR and a second time when we perform a full verification of the
54+
// IR. For testing purposes, we just want the later.
55+
llvm::cl::opt<bool> IsSILOwnershipVerifierTestingEnabled(
56+
"sil-ownership-verifier-enable-testing",
57+
llvm::cl::desc("Put the sil ownership verifier in testing mode. See "
58+
"comment in SILOwnershipVerifier.cpp above option for more "
59+
"information."));
4960

5061
//===----------------------------------------------------------------------===//
5162
// Utility
@@ -116,7 +127,7 @@ class OwnershipCompatibilityUseChecker
116127
<< "Have operand with incompatible ownership?!\n"
117128
<< "Value: " << *getValue() << "User: " << *User
118129
<< "Conv: " << getOwnershipKind() << "\n\n";
119-
if (PrintMessageInsteadOfAssert)
130+
if (IsSILOwnershipVerifierTestingEnabled)
120131
return false;
121132
llvm_unreachable("triggering standard assertion failure routine");
122133
}
@@ -760,7 +771,7 @@ static bool checkFunctionArgWithoutLifetimeEndingUses(SILFunctionArgument *Arg,
760771
<< " Owned function parameter without life "
761772
"ending uses!\n"
762773
<< "Value: " << *Arg << '\n';
763-
if (PrintMessageInsteadOfAssert)
774+
if (IsSILOwnershipVerifierTestingEnabled)
764775
return true;
765776
llvm_unreachable("triggering standard assertion failure routine");
766777
}
@@ -779,7 +790,7 @@ bool SILValueOwnershipChecker::checkValueWithoutLifetimeEndingUses() {
779790
"guaranteed function args must have at least one "
780791
"lifetime ending use?!\n"
781792
<< "Value: " << *Value << '\n';
782-
if (PrintMessageInsteadOfAssert)
793+
if (IsSILOwnershipVerifierTestingEnabled)
783794
return true;
784795
llvm_unreachable("triggering standard assertion failure routine");
785796
}
@@ -800,7 +811,7 @@ static bool isGuaranteedFunctionArgWithLifetimeEndingUses(
800811
llvm::errs() << " Lifetime Ending User: " << *U;
801812
}
802813
llvm::errs() << '\n';
803-
if (PrintMessageInsteadOfAssert)
814+
if (IsSILOwnershipVerifierTestingEnabled)
804815
return false;
805816
llvm_unreachable("triggering standard assertion failure routine");
806817
}
@@ -870,14 +881,14 @@ bool SILValueOwnershipChecker::checkUses() {
870881
// If the block does over consume, we either assert or return false. We only
871882
// return false when debugging.
872883
if (doesBlockDoubleConsume(UserBlock, User, true)) {
873-
if (PrintMessageInsteadOfAssert)
884+
if (IsSILOwnershipVerifierTestingEnabled)
874885
return false;
875886
llvm_unreachable("triggering standard assertion failure routine");
876887
}
877888

878889
// Then check if the given block has a use after free.
879890
if (doesBlockContainUseAfterFree(User, UserBlock)) {
880-
if (PrintMessageInsteadOfAssert)
891+
if (IsSILOwnershipVerifierTestingEnabled)
881892
return false;
882893
llvm_unreachable("triggering standard assertion failure routine");
883894
}
@@ -907,7 +918,7 @@ bool SILValueOwnershipChecker::checkUses() {
907918
// Make sure that the predecessor is not in our
908919
// BlocksWithLifetimeEndingUses list.
909920
if (doesBlockDoubleConsume(PredBlock, User)) {
910-
if (PrintMessageInsteadOfAssert)
921+
if (IsSILOwnershipVerifierTestingEnabled)
911922
return false;
912923
llvm_unreachable("triggering standard assertion failure routine");
913924
}
@@ -965,7 +976,7 @@ void SILValueOwnershipChecker::checkDataflow() {
965976
// 2. We add the predecessor to the worklist if we have not visited it yet.
966977
for (auto *PredBlock : BB->getPredecessorBlocks()) {
967978
if (doesBlockDoubleConsume(PredBlock)) {
968-
if (PrintMessageInsteadOfAssert)
979+
if (IsSILOwnershipVerifierTestingEnabled)
969980
return;
970981
llvm_unreachable("triggering standard assertion failure routine");
971982
}
@@ -989,7 +1000,7 @@ void SILValueOwnershipChecker::checkDataflow() {
9891000
llvm::errs() << "bb" << BB->getDebugID();
9901001
}
9911002
llvm::errs() << '\n';
992-
if (PrintMessageInsteadOfAssert)
1003+
if (IsSILOwnershipVerifierTestingEnabled)
9931004
return;
9941005
llvm_unreachable("triggering standard assertion failure routine");
9951006
}
@@ -1006,7 +1017,7 @@ void SILValueOwnershipChecker::checkDataflow() {
10061017
llvm::errs() << "User:" << *Pair.second << "Block: bb"
10071018
<< Pair.first->getDebugID() << "\n\n";
10081019
}
1009-
if (PrintMessageInsteadOfAssert)
1020+
if (IsSILOwnershipVerifierTestingEnabled)
10101021
return;
10111022
llvm_unreachable("triggering standard assertion failure routine");
10121023
}
@@ -1023,6 +1034,10 @@ void SILInstruction::verifyOperandOwnership() const {
10231034
// If SILOwnership is not enabled, do not perform verification.
10241035
if (!getModule().getOptions().EnableSILOwnership)
10251036
return;
1037+
// If we are testing the verifier, bail so we only print errors once when
1038+
// performing a full verification, instead of additionally in the SILBuilder.
1039+
if (IsSILOwnershipVerifierTestingEnabled)
1040+
return;
10261041
auto *Self = const_cast<SILInstruction *>(this);
10271042
for (const Operand &Op : getAllOperands()) {
10281043
OwnershipCompatibilityUseChecker(getModule(), Op).check(Self);

test/SIL/ownership-verifier/over_consume.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-sil-opt -enable-sil-ownership -sil-ownership-verifier-do-not-assert -enable-sil-verify-all=0 -o /dev/null 2>&1 %s | %FileCheck %s
1+
// RUN: %target-sil-opt -enable-sil-ownership -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 -o /dev/null 2>&1 %s | %FileCheck %s
22
// REQUIRES: asserts
33

44
sil_stage canonical

0 commit comments

Comments
 (0)