Skip to content

Commit 84232cf

Browse files
gottesmmgottesmm
committed
[ownership] Replace all uses outside of the SILOwnershipVerifier of LinearLifetimeChecker::checkValue() and make it a private implementation detail.
HOW THIS WAS DONE: I did this by refactoring the last usages of checkValue into a higher level API that uses checkValue as an implementation detail: completeConsumingUseSet(...). All of these places in MandatoryInlining/PredictableMemOpts all wanted behavior where we complete a set of consuming uses for a value, detecting if the consuming use is in a different loop nest from the value. WHY DO THIS: The reason why I wanted to do this is that checkValue is a lower level API that drives the actual low level computation. We shouldn't expose its interface to the LinearLifetimeChecker's users since it is our own private implementation detail that also has some sharp edges. AN ADDITIONAL BENEFIT: Additionally by hiding the declaration of checkValue, the last public use of LinearLifetimeError and ErrorBehaviorKind was not private. This allowed me to then move the declarations of those two to a private header (./lib/SIL/LinearLifetimeCheckerPrivate.h) and make their declarations private to LinearLifetimeChecker as well. As such, I renamed them to LinearLifetimeChecker::Error and LinearLifetimeChecker::ErrorBehaviorKind.
1 parent 723b2d2 commit 84232cf

File tree

7 files changed

+322
-237
lines changed

7 files changed

+322
-237
lines changed

‎include/swift/SIL/LinearLifetimeChecker.h

Lines changed: 44 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -28,103 +28,8 @@ class SILInstruction;
2828
class SILModule;
2929
class SILValue;
3030
class DeadEndBlocks;
31-
32-
namespace ownership {
33-
34-
struct ErrorBehaviorKind {
35-
enum inner_t {
36-
Invalid = 0,
37-
ReturnFalse = 1,
38-
PrintMessage = 2,
39-
Assert = 4,
40-
ReturnFalseOnLeak = 8,
41-
PrintMessageAndReturnFalse = PrintMessage | ReturnFalse,
42-
PrintMessageAndAssert = PrintMessage | Assert,
43-
ReturnFalseOnLeakAssertOtherwise = ReturnFalseOnLeak | Assert,
44-
} Value;
45-
46-
ErrorBehaviorKind() : Value(Invalid) {}
47-
ErrorBehaviorKind(inner_t Inner) : Value(Inner) { assert(Value != Invalid); }
48-
49-
bool shouldAssert() const {
50-
assert(Value != Invalid);
51-
return Value & Assert;
52-
}
53-
54-
bool shouldReturnFalseOnLeak() const {
55-
assert(Value != Invalid);
56-
return Value & ReturnFalseOnLeak;
57-
}
58-
59-
bool shouldPrintMessage() const {
60-
assert(Value != Invalid);
61-
return Value & PrintMessage;
62-
}
63-
64-
bool shouldReturnFalse() const {
65-
assert(Value != Invalid);
66-
return Value & ReturnFalse;
67-
}
68-
};
69-
70-
} // end namespace ownership
71-
72-
class LinearLifetimeError {
73-
ownership::ErrorBehaviorKind errorBehavior;
74-
bool foundUseAfterFree = false;
75-
bool foundLeak = false;
76-
bool foundOverConsume = false;
77-
78-
public:
79-
LinearLifetimeError(ownership::ErrorBehaviorKind errorBehavior)
80-
: errorBehavior(errorBehavior) {}
81-
82-
bool getFoundError() const {
83-
return foundUseAfterFree || foundLeak || foundOverConsume;
84-
}
85-
86-
bool getFoundLeak() const { return foundLeak; }
87-
88-
bool getFoundUseAfterFree() const { return foundUseAfterFree; }
89-
90-
bool getFoundOverConsume() const { return foundOverConsume; }
91-
92-
void handleLeak(llvm::function_ref<void()> &&messagePrinterFunc) {
93-
foundLeak = true;
94-
95-
if (errorBehavior.shouldPrintMessage())
96-
messagePrinterFunc();
97-
98-
if (errorBehavior.shouldReturnFalseOnLeak())
99-
return;
100-
101-
// We already printed out our error if we needed to, so don't pass it along.
102-
handleError([]() {});
103-
}
104-
105-
void handleOverConsume(llvm::function_ref<void()> &&messagePrinterFunc) {
106-
foundOverConsume = true;
107-
handleError(std::move(messagePrinterFunc));
108-
}
109-
110-
void handleUseAfterFree(llvm::function_ref<void()> &&messagePrinterFunc) {
111-
foundUseAfterFree = true;
112-
handleError(std::move(messagePrinterFunc));
113-
}
114-
115-
private:
116-
void handleError(llvm::function_ref<void()> &&messagePrinterFunc) {
117-
if (errorBehavior.shouldPrintMessage())
118-
messagePrinterFunc();
119-
120-
if (errorBehavior.shouldReturnFalse()) {
121-
return;
122-
}
123-
124-
assert(errorBehavior.shouldAssert() && "At this point, we should assert");
125-
llvm_unreachable("triggering standard assertion failure routine");
126-
}
127-
};
31+
class SILOwnershipVerifier;
32+
class SILValueOwnershipChecker;
12833

12934
/// A class used to validate linear lifetime with respect to an SSA-like
13035
/// definition.
@@ -140,6 +45,14 @@ class LinearLifetimeError {
14045
/// uses must not be reachable from each other and jointly post-dominate all
14146
/// consuming uses as well as the defining block/instruction.
14247
class LinearLifetimeChecker {
48+
public:
49+
class Error;
50+
struct ErrorBehaviorKind;
51+
52+
private:
53+
friend class SILOwnershipVerifier;
54+
friend class SILValueOwnershipChecker;
55+
14356
SmallPtrSetImpl<SILBasicBlock *> &visitedBlocks;
14457
DeadEndBlocks &deadEndBlocks;
14558

@@ -148,6 +61,36 @@ class LinearLifetimeChecker {
14861
DeadEndBlocks &deadEndBlocks)
14962
: visitedBlocks(visitedBlocks), deadEndBlocks(deadEndBlocks) {}
15063

64+
/// Returns true that \p value forms a linear lifetime with consuming uses \p
65+
/// consumingUses, non consuming uses \p nonConsumingUses. Returns false
66+
/// otherwise.
67+
bool validateLifetime(SILValue value, ArrayRef<Operand *> consumingUses,
68+
ArrayRef<Operand *> nonConsumingUses);
69+
70+
/// Given a value and a consuming use of that value, compute a non-unique
71+
/// minimal set of insertion points that together with \p consumingUse
72+
/// post-dominate and end the lifetime of \p value.
73+
///
74+
/// Returns true if we completed the consuming use set and discovered that \p
75+
/// consumingUse is not strongly control equivalent to value (meaning
76+
/// consumingUse is not in the same loop in the loop nest as value).
77+
///
78+
/// \p scratch A scratch buffer to be used during our computation. If nullptr,
79+
/// routine will create its own small vector.
80+
bool completeConsumingUseSet(
81+
SILValue value, Operand *consumingUse,
82+
SmallVectorImpl<SILBasicBlock *> &scratch,
83+
function_ref<void(SILBasicBlock::iterator insertPt)> visitor);
84+
85+
/// Overload without scratch space.
86+
bool completeConsumingUseSet(
87+
SILValue value, Operand *consumingUse,
88+
function_ref<void(SILBasicBlock::iterator insertPt)> visitor) {
89+
SmallVector<SILBasicBlock *, 4> scratch;
90+
return completeConsumingUseSet(value, consumingUse, scratch, visitor);
91+
}
92+
93+
private:
15194
/// Returns true if:
15295
///
15396
/// 1. No consuming uses are reachable from any other consuming use, from any
@@ -164,22 +107,10 @@ class LinearLifetimeChecker {
164107
/// error.
165108
/// \p leakingBlocks If non-null a list of blocks where the value was detected
166109
/// to leak. Can be used to insert missing destroys.
167-
LinearLifetimeError
168-
checkValue(SILValue value, ArrayRef<Operand *> consumingUses,
169-
ArrayRef<Operand *> nonConsumingUses,
170-
ownership::ErrorBehaviorKind errorBehavior,
171-
SmallVectorImpl<SILBasicBlock *> *leakingBlocks = nullptr);
172-
173-
/// Returns true that \p value forms a linear lifetime with consuming uses \p
174-
/// consumingUses, non consuming uses \p nonConsumingUses. Returns false
175-
/// otherwise.
176-
bool validateLifetime(SILValue value, ArrayRef<Operand *> consumingUses,
177-
ArrayRef<Operand *> nonConsumingUses) {
178-
return !checkValue(value, consumingUses, nonConsumingUses,
179-
ownership::ErrorBehaviorKind::ReturnFalse,
180-
nullptr /*leakingBlocks*/)
181-
.getFoundError();
182-
}
110+
Error checkValue(SILValue value, ArrayRef<Operand *> consumingUses,
111+
ArrayRef<Operand *> nonConsumingUses,
112+
ErrorBehaviorKind errorBehavior,
113+
SmallVectorImpl<SILBasicBlock *> *leakingBlocks = nullptr);
183114
};
184115

185116
} // namespace swift

‎lib/SIL/LinearLifetimeChecker.cpp

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
//===----------------------------------------------------------------------===//
2222

2323
#define DEBUG_TYPE "sil-linear-lifetime-checker"
24-
#include "swift/SIL/LinearLifetimeChecker.h"
24+
#include "LinearLifetimeCheckerPrivate.h"
2525
#include "swift/SIL/BasicBlockUtils.h"
2626
#include "swift/SIL/OwnershipUtils.h"
2727
#include "swift/SIL/SILBasicBlock.h"
@@ -31,7 +31,6 @@
3131
#include "llvm/Support/Debug.h"
3232

3333
using namespace swift;
34-
using namespace swift::ownership;
3534

3635
//===----------------------------------------------------------------------===//
3736
// Declarations
@@ -51,7 +50,7 @@ struct State {
5150

5251
/// The result error object that use to signal either that no errors were
5352
/// found or if errors are found the specific type of error that was found.
54-
LinearLifetimeError error;
53+
LinearLifetimeChecker::Error error;
5554

5655
/// The blocks that we have already visited.
5756
SmallPtrSetImpl<SILBasicBlock *> &visitedBlocks;
@@ -82,7 +81,7 @@ struct State {
8281
SmallSetVector<SILBasicBlock *, 8> successorBlocksThatMustBeVisited;
8382

8483
State(SILValue value, SmallPtrSetImpl<SILBasicBlock *> &visitedBlocks,
85-
ErrorBehaviorKind errorBehavior,
84+
LinearLifetimeChecker::ErrorBehaviorKind errorBehavior,
8685
SmallVectorImpl<SILBasicBlock *> *leakingBlocks,
8786
ArrayRef<Operand *> consumingUses, ArrayRef<Operand *> nonConsumingUses)
8887
: value(value), beginBlock(value->getParentBlock()), error(errorBehavior),
@@ -91,7 +90,7 @@ struct State {
9190

9291
State(SILBasicBlock *beginBlock,
9392
SmallPtrSetImpl<SILBasicBlock *> &visitedBlocks,
94-
ErrorBehaviorKind errorBehavior,
93+
LinearLifetimeChecker::ErrorBehaviorKind errorBehavior,
9594
SmallVectorImpl<SILBasicBlock *> *leakingBlocks,
9695
ArrayRef<Operand *> consumingUses, ArrayRef<Operand *> nonConsumingUses)
9796
: value(), beginBlock(beginBlock), error(errorBehavior),
@@ -495,7 +494,7 @@ void State::checkDataflowEndState(DeadEndBlocks &deBlocks) {
495494
// Top Level Entrypoints
496495
//===----------------------------------------------------------------------===//
497496

498-
LinearLifetimeError LinearLifetimeChecker::checkValue(
497+
LinearLifetimeChecker::Error LinearLifetimeChecker::checkValue(
499498
SILValue value, ArrayRef<Operand *> consumingUses,
500499
ArrayRef<Operand *> nonConsumingUses, ErrorBehaviorKind errorBehavior,
501500
SmallVectorImpl<SILBasicBlock *> *leakingBlocks) {
@@ -588,3 +587,30 @@ LinearLifetimeError LinearLifetimeChecker::checkValue(
588587
state.checkDataflowEndState(deadEndBlocks);
589588
return state.error;
590589
}
590+
591+
bool LinearLifetimeChecker::completeConsumingUseSet(
592+
SILValue value, Operand *consumingUse,
593+
SmallVectorImpl<SILBasicBlock *> &scratch,
594+
function_ref<void(SILBasicBlock::iterator)> visitor) {
595+
auto error = checkValue(value, {consumingUse}, {},
596+
ErrorBehaviorKind::ReturnFalse, &scratch);
597+
598+
if (!error.getFoundError()) {
599+
return false;
600+
}
601+
602+
while (!scratch.empty()) {
603+
visitor(scratch.pop_back_val()->begin());
604+
}
605+
606+
// Return true if we found an over consume (meaning our use is in a loop).
607+
return error.getFoundOverConsume();
608+
}
609+
610+
bool LinearLifetimeChecker::validateLifetime(
611+
SILValue value, ArrayRef<Operand *> consumingUses,
612+
ArrayRef<Operand *> nonConsumingUses) {
613+
return !checkValue(value, consumingUses, nonConsumingUses,
614+
ErrorBehaviorKind::ReturnFalse, nullptr /*leakingBlocks*/)
615+
.getFoundError();
616+
}

‎lib/SIL/LinearLifetimeCheckerPrivate.h

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
//===--- LinearLifetimeCheckerPrivate.h -----------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#ifndef SWIFT_SIL_LINEARLIFETIMECHECKER_PRIVATE_H
14+
#define SWIFT_SIL_LINEARLIFETIMECHECKER_PRIVATE_H
15+
16+
#include "swift/SIL/LinearLifetimeChecker.h"
17+
18+
namespace swift {
19+
20+
struct LinearLifetimeChecker::ErrorBehaviorKind {
21+
enum inner_t {
22+
Invalid = 0,
23+
ReturnFalse = 1,
24+
PrintMessage = 2,
25+
Assert = 4,
26+
ReturnFalseOnLeak = 8,
27+
PrintMessageAndReturnFalse = PrintMessage | ReturnFalse,
28+
PrintMessageAndAssert = PrintMessage | Assert,
29+
ReturnFalseOnLeakAssertOtherwise = ReturnFalseOnLeak | Assert,
30+
} Value;
31+
32+
ErrorBehaviorKind() : Value(Invalid) {}
33+
ErrorBehaviorKind(inner_t Inner) : Value(Inner) { assert(Value != Invalid); }
34+
35+
bool shouldAssert() const {
36+
assert(Value != Invalid);
37+
return Value & Assert;
38+
}
39+
40+
bool shouldReturnFalseOnLeak() const {
41+
assert(Value != Invalid);
42+
return Value & ReturnFalseOnLeak;
43+
}
44+
45+
bool shouldPrintMessage() const {
46+
assert(Value != Invalid);
47+
return Value & PrintMessage;
48+
}
49+
50+
bool shouldReturnFalse() const {
51+
assert(Value != Invalid);
52+
return Value & ReturnFalse;
53+
}
54+
};
55+
56+
class LinearLifetimeChecker::Error {
57+
ErrorBehaviorKind errorBehavior;
58+
bool foundUseAfterFree = false;
59+
bool foundLeak = false;
60+
bool foundOverConsume = false;
61+
62+
public:
63+
Error(ErrorBehaviorKind errorBehavior) : errorBehavior(errorBehavior) {}
64+
65+
bool getFoundError() const {
66+
return foundUseAfterFree || foundLeak || foundOverConsume;
67+
}
68+
69+
bool getFoundLeak() const { return foundLeak; }
70+
71+
bool getFoundUseAfterFree() const { return foundUseAfterFree; }
72+
73+
bool getFoundOverConsume() const { return foundOverConsume; }
74+
75+
void handleLeak(llvm::function_ref<void()> &&messagePrinterFunc) {
76+
foundLeak = true;
77+
78+
if (errorBehavior.shouldPrintMessage())
79+
messagePrinterFunc();
80+
81+
if (errorBehavior.shouldReturnFalseOnLeak())
82+
return;
83+
84+
// We already printed out our error if we needed to, so don't pass it along.
85+
handleError([]() {});
86+
}
87+
88+
void handleOverConsume(llvm::function_ref<void()> &&messagePrinterFunc) {
89+
foundOverConsume = true;
90+
handleError(std::move(messagePrinterFunc));
91+
}
92+
93+
void handleUseAfterFree(llvm::function_ref<void()> &&messagePrinterFunc) {
94+
foundUseAfterFree = true;
95+
handleError(std::move(messagePrinterFunc));
96+
}
97+
98+
private:
99+
void handleError(llvm::function_ref<void()> &&messagePrinterFunc) {
100+
if (errorBehavior.shouldPrintMessage())
101+
messagePrinterFunc();
102+
103+
if (errorBehavior.shouldReturnFalse()) {
104+
return;
105+
}
106+
107+
assert(errorBehavior.shouldAssert() && "At this point, we should assert");
108+
llvm_unreachable("triggering standard assertion failure routine");
109+
}
110+
};
111+
112+
} // namespace swift
113+
114+
#endif

0 commit comments

Comments
 (0)