Skip to content

Commit 2c9a34f

Browse files
gottesmmgottesmm
committed
[opt-remark] Teach opt-remark how to emit notes that point to the value being retained/released.
This is just an initial implementation and doesn't handle all cases. This works by grabbing the rc-identity root of a retain/release operation and then seeing: 1. If it is an argument, we use the ValueDecl of the argument. 2. If it is an instruction result, we try to infer the ValueDecl by looking for debug_value uses. 3. In the future, we should add support for globals and looking at addresses. I may do 3 since it is important to have support for that due to inout objects.
1 parent aea6d7d commit 2c9a34f

File tree

5 files changed

+173
-45
lines changed

5 files changed

+173
-45
lines changed

‎include/swift/SIL/OptimizationRemark.h

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include "swift/Basic/SourceLoc.h"
2323
#include "swift/Demangling/Demangler.h"
24+
#include "swift/SIL/SILArgument.h"
2425
#include "swift/SIL/SILBasicBlock.h"
2526
#include "swift/SIL/SILInstruction.h"
2627
#include "swift/SIL/SILModule.h"
@@ -95,11 +96,12 @@ struct Argument {
9596
/// If set, the debug location corresponding to the value.
9697
SourceLoc loc;
9798

98-
explicit Argument(StringRef Str = "")
99-
: key(ArgumentKeyKind::Default, "String"), val(Str) {}
100-
Argument(StringRef key, StringRef val)
101-
: key(ArgumentKeyKind::Default, key), val(val) {}
99+
explicit Argument(StringRef str = "")
100+
: Argument({ArgumentKeyKind::Default, "String"}, str) {}
102101

102+
Argument(StringRef key, StringRef val)
103+
: Argument({ArgumentKeyKind::Default, key}, val) {}
104+
Argument(ArgumentKey key, StringRef val) : key(key), val(val) {}
103105
Argument(StringRef key, int n);
104106
Argument(StringRef key, long n);
105107
Argument(StringRef key, long long n);
@@ -112,6 +114,24 @@ struct Argument {
112114
Argument(ArgumentKey key, SILFunction *f);
113115
Argument(StringRef key, SILType ty);
114116
Argument(StringRef key, CanType ty);
117+
118+
Argument(StringRef key, StringRef msg, const ValueDecl *decl)
119+
: Argument(ArgumentKey(ArgumentKeyKind::Default, key), msg, decl) {}
120+
Argument(ArgumentKey key, StringRef msg, const ValueDecl *decl)
121+
: key(key), val(msg), loc(decl->getLoc()) {}
122+
123+
/// Given a value, call \p funcPassedInferredArgs for each associated
124+
/// ValueDecl that is associated with \p value. All created Arguments are
125+
/// passed the same StringRef. To stop iteration, return false in \p
126+
/// funcPassedInferedArgs.
127+
///
128+
/// NOTE: the function may be called multiple times if the optimizer joined
129+
/// two live ranges and thus when iterating over value's users we see multiple
130+
/// debug_value operations.
131+
static bool
132+
inferArgumentsForValue(ArgumentKeyKind keyKind, StringRef message,
133+
SILValue value,
134+
function_ref<bool(Argument)> funcPassedInferedArgs);
115135
};
116136

117137
/// Shorthand to insert named-value pairs.

‎lib/SIL/Utils/OptimizationRemark.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
#include "swift/AST/DiagnosticEngine.h"
2121
#include "swift/AST/DiagnosticsSIL.h"
2222
#include "swift/Demangling/Demangler.h"
23+
#include "swift/SIL/DebugUtils.h"
24+
#include "swift/SIL/SILArgument.h"
25+
#include "swift/SIL/SILInstruction.h"
2326
#include "swift/SIL/SILRemarkStreamer.h"
2427
#include "llvm/ADT/StringExtras.h"
2528
#include "llvm/Support/CommandLine.h"
@@ -72,6 +75,28 @@ Argument::Argument(StringRef key, CanType ty)
7275
ty.print(stream);
7376
}
7477

78+
bool Argument::inferArgumentsForValue(
79+
ArgumentKeyKind keyKind, StringRef msg, SILValue value,
80+
function_ref<bool(Argument)> funcPassedInferedArgs) {
81+
// If we have an argument, just use that.
82+
if (auto *arg = dyn_cast<SILArgument>(value))
83+
if (auto *decl = arg->getDecl())
84+
return funcPassedInferedArgs(
85+
Argument({keyKind, "InferredValue"}, msg, decl));
86+
87+
// TODO: Look for loads from globals and addresses.
88+
89+
// Otherwise, look for debug_values.
90+
for (auto *use : getDebugUses(value))
91+
if (auto *dvi = dyn_cast<DebugValueInst>(use->getUser()))
92+
if (auto *decl = dvi->getDecl())
93+
if (!funcPassedInferedArgs(
94+
Argument({keyKind, "InferredValue"}, msg, decl)))
95+
return false;
96+
97+
return true;
98+
}
99+
75100
template <typename DerivedT>
76101
std::string Remark<DerivedT>::getMsg() const {
77102
std::string str;

‎lib/SILOptimizer/Transforms/OptRemarkGenerator.cpp

Lines changed: 91 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "swift/SIL/SILInstruction.h"
1818
#include "swift/SIL/SILModule.h"
1919
#include "swift/SIL/SILVisitor.h"
20+
#include "swift/SILOptimizer/Analysis/RCIdentityAnalysis.h"
2021
#include "swift/SILOptimizer/PassManager/Passes.h"
2122
#include "swift/SILOptimizer/PassManager/Transforms.h"
2223

@@ -31,10 +32,12 @@ namespace {
3132
struct OptRemarkGeneratorInstructionVisitor
3233
: public SILInstructionVisitor<OptRemarkGeneratorInstructionVisitor> {
3334
SILModule &mod;
35+
RCIdentityFunctionInfo &rcfi;
3436
OptRemark::Emitter ORE;
3537

36-
OptRemarkGeneratorInstructionVisitor(SILModule &mod)
37-
: mod(mod), ORE(DEBUG_TYPE, mod) {}
38+
OptRemarkGeneratorInstructionVisitor(SILModule &mod,
39+
RCIdentityFunctionInfo &rcfi)
40+
: mod(mod), rcfi(rcfi), ORE(DEBUG_TYPE, mod) {}
3841

3942
void visitStrongRetainInst(StrongRetainInst *sri);
4043
void visitStrongReleaseInst(StrongReleaseInst *sri);
@@ -49,10 +52,28 @@ void OptRemarkGeneratorInstructionVisitor::visitStrongRetainInst(
4952
StrongRetainInst *sri) {
5053
ORE.emit([&]() {
5154
using namespace OptRemark;
55+
SILValue root = rcfi.getRCIdentityRoot(sri->getOperand());
56+
SmallVector<Argument, 8> inferredArgs;
57+
bool foundArgs = Argument::inferArgumentsForValue(
58+
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
59+
inferredArgs.push_back(arg);
60+
return true;
61+
});
62+
(void)foundArgs;
63+
5264
// Retains begin a lifetime scope so we infer scan forward.
53-
return RemarkMissed("memory-management", *sri,
54-
SourceLocInferenceBehavior::ForwardScanOnly)
55-
<< "Unable to remove retain";
65+
auto remark = RemarkMissed("memory-management", *sri,
66+
SourceLocInferenceBehavior::ForwardScanOnly)
67+
<< "Found retain:";
68+
if (inferredArgs.empty()) {
69+
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
70+
"Unable to infer any values being retained.");
71+
} else {
72+
for (auto arg : inferredArgs) {
73+
remark << arg;
74+
}
75+
}
76+
return remark;
5677
});
5778
}
5879

@@ -61,31 +82,85 @@ void OptRemarkGeneratorInstructionVisitor::visitStrongReleaseInst(
6182
ORE.emit([&]() {
6283
using namespace OptRemark;
6384
// Releases end a lifetime scope so we infer scan backward.
64-
return RemarkMissed("memory-management", *sri,
65-
SourceLocInferenceBehavior::BackwardScanOnly)
66-
<< "Unable to remove release";
85+
SILValue root = rcfi.getRCIdentityRoot(sri->getOperand());
86+
SmallVector<Argument, 8> inferredArgs;
87+
bool foundArgs = Argument::inferArgumentsForValue(
88+
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
89+
inferredArgs.push_back(arg);
90+
return true;
91+
});
92+
(void)foundArgs;
93+
auto remark = RemarkMissed("memory-management", *sri,
94+
SourceLocInferenceBehavior::BackwardScanOnly)
95+
<< "Found release:";
96+
if (inferredArgs.empty()) {
97+
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
98+
"Unable to infer any values being released.");
99+
} else {
100+
for (auto arg : inferredArgs) {
101+
remark << arg;
102+
}
103+
}
104+
return remark;
67105
});
68106
}
69107

70108
void OptRemarkGeneratorInstructionVisitor::visitRetainValueInst(
71109
RetainValueInst *rvi) {
72110
ORE.emit([&]() {
73111
using namespace OptRemark;
112+
SILValue root = rcfi.getRCIdentityRoot(rvi->getOperand());
113+
SmallVector<Argument, 8> inferredArgs;
114+
bool foundArgs = Argument::inferArgumentsForValue(
115+
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
116+
inferredArgs.push_back(arg);
117+
return true;
118+
});
119+
(void)foundArgs;
120+
74121
// Retains begin a lifetime scope, so we infer scan forwards.
75-
return RemarkMissed("memory-management", *rvi,
76-
SourceLocInferenceBehavior::ForwardScanOnly)
77-
<< "Unable to remove retain";
122+
auto remark = RemarkMissed("memory-management", *rvi,
123+
SourceLocInferenceBehavior::ForwardScanOnly)
124+
<< "Found retain:";
125+
if (inferredArgs.empty()) {
126+
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
127+
"Unable to infer any values being retained.");
128+
} else {
129+
for (auto arg : inferredArgs) {
130+
remark << arg;
131+
}
132+
}
133+
return remark;
78134
});
79135
}
80136

81137
void OptRemarkGeneratorInstructionVisitor::visitReleaseValueInst(
82138
ReleaseValueInst *rvi) {
83139
ORE.emit([&]() {
84140
using namespace OptRemark;
141+
SILValue root = rcfi.getRCIdentityRoot(rvi->getOperand());
142+
SmallVector<Argument, 8> inferredArgs;
143+
bool foundArgs = Argument::inferArgumentsForValue(
144+
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
145+
inferredArgs.push_back(arg);
146+
return true;
147+
});
148+
(void)foundArgs;
149+
85150
// Releases end a lifetime scope so we infer scan backward.
86-
return RemarkMissed("memory-management", *rvi,
87-
SourceLocInferenceBehavior::BackwardScanOnly)
88-
<< "Unable to remove release";
151+
auto remark = RemarkMissed("memory-management", *rvi,
152+
SourceLocInferenceBehavior::BackwardScanOnly)
153+
<< "Found release:";
154+
if (inferredArgs.empty()) {
155+
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
156+
"Unable to infer any values being released.");
157+
} else {
158+
for (auto arg : inferredArgs) {
159+
remark << arg;
160+
}
161+
}
162+
163+
return remark;
89164
});
90165
}
91166

@@ -115,7 +190,8 @@ class OptRemarkGenerator : public SILFunctionTransform {
115190

116191
auto *fn = getFunction();
117192
LLVM_DEBUG(llvm::dbgs() << "Visiting: " << fn->getName() << "\n");
118-
OptRemarkGeneratorInstructionVisitor visitor(fn->getModule());
193+
auto &rcfi = *getAnalysis<RCIdentityAnalysis>()->get(fn);
194+
OptRemarkGeneratorInstructionVisitor visitor(fn->getModule(), rcfi);
119195
for (auto &block : *fn) {
120196
for (auto &inst : block) {
121197
visitor.visit(&inst);

‎test/SILOptimizer/opt-remark-generator.sil

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,19 @@
1-
// RUN: %target-sil-opt -sil-opt-remark-generator -sil-remarks-missed=sil-opt-remark-gen %s -o /dev/null 2>&1 | %FileCheck %s
2-
3-
// CHECK: {{.*}}opt-remark-generator.sil:18:3: remark: Unable to remove retain
4-
// CHECK-NEXT: strong_retain
5-
// CHECK: {{.*}}opt-remark-generator.sil:19:3: remark: Unable to remove retain
6-
// CHECK-NEXT: retain_value
7-
// CHECK: {{.*}}opt-remark-generator.sil:20:3: remark: Unable to remove release
8-
// CHECK-NEXT: strong_release
9-
// CHECK: {{.*}}opt-remark-generator.sil:21:3: remark: Unable to remove release
10-
// CHECK-NEXT: release_value
1+
// RUN: %target-sil-opt -sil-opt-remark-generator -sil-remarks-missed=sil-opt-remark-gen -verify %s -o /dev/null
112

123
sil_stage canonical
134

145
import Builtin
156

167
sil @foo : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
178
bb0(%0 : $Builtin.NativeObject):
18-
strong_retain %0 : $Builtin.NativeObject
19-
retain_value %0 : $Builtin.NativeObject
20-
strong_release %0 : $Builtin.NativeObject
21-
release_value %0 : $Builtin.NativeObject
9+
strong_retain %0 : $Builtin.NativeObject // expected-remark {{Found retain:}}
10+
// expected-note @-1 {{Unable to infer any values being retained.}}
11+
retain_value %0 : $Builtin.NativeObject // expected-remark {{Found retain:}}
12+
// expected-note @-1 {{Unable to infer any values being retained.}}
13+
strong_release %0 : $Builtin.NativeObject // expected-remark {{Found release:}}
14+
// expected-note @-1 {{Unable to infer any values being released.}}
15+
release_value %0 : $Builtin.NativeObject // expected-remark {{Found release:}}
16+
// expected-note @-1 {{Unable to infer any values being released.}}
2217
%9999 = tuple()
2318
return %9999 : $()
2419
}

‎test/SILOptimizer/opt-remark-generator.swift

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,37 +7,45 @@
77
// CHECK-NEXT: Pass: sil-opt-remark-gen
88
// CHECK-NEXT: Name: sil.memory-management
99
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
10-
// CHECK-NEXT: Line: 49, Column: 5 }
10+
// CHECK-NEXT: Line: 57, Column: 5 }
1111
// CHECK-NEXT: Function: 'getGlobal()'
1212
// CHECK-NEXT: Args:
13-
// CHECK-NEXT: - String: Unable to remove retain
13+
// CHECK-NEXT: - String: 'Found retain:'
14+
// CHECK-NEXT: - InferValueFailure: Unable to infer any values being retained.
1415
// CHECK-NEXT: ...
1516
// CHECK-NEXT: --- !Missed
1617
// CHECK-NEXT: Pass: sil-opt-remark-gen
1718
// CHECK-NEXT: Name: sil.memory-management
1819
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
19-
// CHECK-NEXT: Line: 56, Column: 5 }
20+
// CHECK-NEXT: Line: 65, Column: 5 }
2021
// CHECK-NEXT: Function: 'useGlobal()'
2122
// CHECK-NEXT: Args:
22-
// CHECK-NEXT: - String: Unable to remove retain
23+
// CHECK-NEXT: - String: 'Found retain:'
24+
// CHECK-NEXT: - InferredValue: 'on value:'
25+
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
26+
// CHECK-NEXT: Line: 62, Column: 9 }
2327
// CHECK-NEXT: ...
2428
// CHECK-NEXT: --- !Missed
2529
// CHECK-NEXT: Pass: sil-opt-remark-gen
2630
// CHECK-NEXT: Name: sil.memory-management
2731
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
28-
// CHECK-NEXT: Line: 56, Column: 12 }
32+
// CHECK-NEXT: Line: 65, Column: 12 }
2933
// CHECK-NEXT: Function: 'useGlobal()'
3034
// CHECK-NEXT: Args:
31-
// CHECK-NEXT: - String: Unable to remove release
35+
// CHECK-NEXT: - String: 'Found release:'
36+
// CHECK-NEXT: - InferValueFailure: Unable to infer any values being released.
3237
// CHECK-NEXT: ...
3338
// CHECK-NEXT: --- !Missed
3439
// CHECK-NEXT: Pass: sil-opt-remark-gen
3540
// CHECK-NEXT: Name: sil.memory-management
3641
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
37-
// CHECK-NEXT: Line: 56, Column: 12 }
42+
// CHECK-NEXT: Line: 65, Column: 12 }
3843
// CHECK-NEXT: Function: 'useGlobal()'
3944
// CHECK-NEXT: Args:
40-
// CHECK-NEXT: - String: Unable to remove release
45+
// CHECK-NEXT: - String: 'Found release:'
46+
// CHECK-NEXT: - InferredValue: 'on value:'
47+
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
48+
// CHECK-NEXT: Line: 62, Column: 9 }
4149
// CHECK-NEXT: ...
4250

4351
public class Klass {}
@@ -46,14 +54,18 @@ public var global = Klass()
4654

4755
@inline(never)
4856
public func getGlobal() -> Klass {
49-
return global // expected-remark @:5 {{Unable to remove retain}}
57+
return global // expected-remark @:5 {{Found retain:}}
58+
// expected-note @-1:5 {{Unable to infer any values being retained.}}
5059
}
5160

5261
public func useGlobal() {
5362
let x = getGlobal()
5463
// Make sure that the retain msg is at the beginning of the print and the
5564
// releases are the end of the print.
56-
print(x) // expected-remark @:5 {{Unable to remove retain}}
57-
// expected-remark @-1:12 {{Unable to remove release}}
58-
// expected-remark @-2:12 {{Unable to remove release}}
65+
print(x) // expected-remark @:5 {{Found retain:}}
66+
// expected-note @-4:9 {{on value:}}
67+
// expected-remark @-2:12 {{Found release:}}
68+
// expected-note @-3:12 {{Unable to infer any values being released.}}
69+
// expected-remark @-4:12 {{Found release:}}
70+
// expected-note @-8:9 {{on value:}}
5971
}

0 commit comments

Comments
 (0)