Skip to content

Commit 2b467ad

Browse files
gottesmmgottesmm
committed
[opt-remark] Clean up remarks a little bit and emit the base name of each decl textually.
So now if one looks at remarks in an IDE, you will see in the NOTE itself the decl to look for. I am going to expand this to include handling of projection paths.
1 parent f2e8a26 commit 2b467ad

File tree

3 files changed

+94
-94
lines changed

3 files changed

+94
-94
lines changed

‎lib/SILOptimizer/Transforms/OptRemarkGenerator.cpp

Lines changed: 71 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "swift/SILOptimizer/Analysis/RCIdentityAnalysis.h"
2222
#include "swift/SILOptimizer/PassManager/Passes.h"
2323
#include "swift/SILOptimizer/PassManager/Transforms.h"
24+
#include "llvm/Support/raw_ostream.h"
2425

2526
using namespace swift;
2627

@@ -42,14 +43,18 @@ struct ValueToDeclInferrer {
4243
using Argument = OptRemark::Argument;
4344
using ArgumentKeyKind = OptRemark::ArgumentKeyKind;
4445

45-
bool infer(ArgumentKeyKind keyKind, StringRef msg, SILValue value,
46+
bool infer(ArgumentKeyKind keyKind, SILValue value,
4647
function_ref<bool(Argument)> funcPassedInferedArgs);
4748
};
4849

4950
} // anonymous namespace
5051

52+
static void printNote(llvm::raw_string_ostream &stream, const ValueDecl *decl) {
53+
stream << "of '" << decl->getBaseName() << "'";
54+
}
55+
5156
bool ValueToDeclInferrer::infer(
52-
ArgumentKeyKind keyKind, StringRef msg, SILValue value,
57+
ArgumentKeyKind keyKind, SILValue value,
5358
function_ref<bool(Argument)> funcPassedInferedArgs) {
5459

5560
// This is a linear IR traversal using a 'falling while loop'. That means
@@ -60,15 +65,27 @@ bool ValueToDeclInferrer::infer(
6065
while (true) {
6166
// First check for "identified values" like arguments and global_addr.
6267
if (auto *arg = dyn_cast<SILArgument>(value))
63-
if (auto *decl = arg->getDecl())
68+
if (auto *decl = arg->getDecl()) {
69+
std::string msg;
70+
{
71+
llvm::raw_string_ostream stream(msg);
72+
printNote(stream, decl);
73+
}
6474
return funcPassedInferedArgs(
65-
Argument({keyKind, "InferredValue"}, msg, decl));
75+
Argument({keyKind, "InferredValue"}, std::move(msg), decl));
76+
}
6677

6778
if (auto *ga = dyn_cast<GlobalAddrInst>(value))
68-
if (auto *decl = ga->getReferencedGlobal()->getDecl())
79+
if (auto *decl = ga->getReferencedGlobal()->getDecl()) {
80+
std::string msg;
81+
{
82+
llvm::raw_string_ostream stream(msg);
83+
printNote(stream, decl);
84+
}
6985
if (!funcPassedInferedArgs(
70-
Argument({keyKind, "InferredValue"}, msg, decl)))
86+
Argument({keyKind, "InferredValue"}, std::move(msg), decl)))
7187
return false;
88+
}
7289

7390
// Then visit our users and see if we can find a debug_value that provides
7491
// us with a decl we can use to construct an argument.
@@ -78,15 +95,23 @@ bool ValueToDeclInferrer::infer(
7895
continue;
7996

8097
if (auto *dvi = dyn_cast<DebugValueInst>(use->getUser())) {
81-
if (auto *decl = dvi->getDecl())
98+
if (auto *decl = dvi->getDecl()) {
99+
std::string msg;
100+
{
101+
llvm::raw_string_ostream stream(msg);
102+
printNote(stream, decl);
103+
}
82104
if (!funcPassedInferedArgs(
83-
Argument({keyKind, "InferredValue"}, msg, decl)))
105+
Argument({keyKind, "InferredValue"}, std::move(msg), decl)))
84106
return false;
107+
}
85108
}
86109
}
87110

88111
// At this point, we could not infer any argument. See if we can look
89-
// through loads, geps to construct a ProjectionPath.
112+
// through loads.
113+
//
114+
// TODO: Add GEPs to construct a ProjectionPath.
90115

91116
// Finally, see if we can look through a load...
92117
if (auto *li = dyn_cast<LoadInst>(value)) {
@@ -135,24 +160,19 @@ void OptRemarkGeneratorInstructionVisitor::visitStrongRetainInst(
135160
using namespace OptRemark;
136161
SILValue root = rcfi.getRCIdentityRoot(sri->getOperand());
137162
SmallVector<Argument, 8> inferredArgs;
138-
bool foundArgs = valueToDeclInferrer.infer(
139-
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
140-
inferredArgs.push_back(arg);
141-
return true;
142-
});
163+
bool foundArgs = valueToDeclInferrer.infer(ArgumentKeyKind::Note, root,
164+
[&](Argument arg) {
165+
inferredArgs.push_back(arg);
166+
return true;
167+
});
143168
(void)foundArgs;
144169

145170
// Retains begin a lifetime scope so we infer scan forward.
146-
auto remark = RemarkMissed("memory-management", *sri,
171+
auto remark = RemarkMissed("memory", *sri,
147172
SourceLocInferenceBehavior::ForwardScanOnly)
148-
<< "Found retain:";
149-
if (inferredArgs.empty()) {
150-
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
151-
"Unable to infer any values being retained.");
152-
} else {
153-
for (auto arg : inferredArgs) {
154-
remark << arg;
155-
}
173+
<< "retain";
174+
for (auto arg : inferredArgs) {
175+
remark << arg;
156176
}
157177
return remark;
158178
});
@@ -165,22 +185,18 @@ void OptRemarkGeneratorInstructionVisitor::visitStrongReleaseInst(
165185
// Releases end a lifetime scope so we infer scan backward.
166186
SILValue root = rcfi.getRCIdentityRoot(sri->getOperand());
167187
SmallVector<Argument, 8> inferredArgs;
168-
bool foundArgs = valueToDeclInferrer.infer(
169-
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
170-
inferredArgs.push_back(arg);
171-
return true;
172-
});
188+
bool foundArgs = valueToDeclInferrer.infer(ArgumentKeyKind::Note, root,
189+
[&](Argument arg) {
190+
inferredArgs.push_back(arg);
191+
return true;
192+
});
173193
(void)foundArgs;
174-
auto remark = RemarkMissed("memory-management", *sri,
194+
195+
auto remark = RemarkMissed("memory", *sri,
175196
SourceLocInferenceBehavior::BackwardScanOnly)
176-
<< "Found release:";
177-
if (inferredArgs.empty()) {
178-
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
179-
"Unable to infer any values being released.");
180-
} else {
181-
for (auto arg : inferredArgs) {
182-
remark << arg;
183-
}
197+
<< "release";
198+
for (auto arg : inferredArgs) {
199+
remark << arg;
184200
}
185201
return remark;
186202
});
@@ -192,24 +208,19 @@ void OptRemarkGeneratorInstructionVisitor::visitRetainValueInst(
192208
using namespace OptRemark;
193209
SILValue root = rcfi.getRCIdentityRoot(rvi->getOperand());
194210
SmallVector<Argument, 8> inferredArgs;
195-
bool foundArgs = valueToDeclInferrer.infer(
196-
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
197-
inferredArgs.push_back(arg);
198-
return true;
199-
});
211+
bool foundArgs = valueToDeclInferrer.infer(ArgumentKeyKind::Note, root,
212+
[&](Argument arg) {
213+
inferredArgs.push_back(arg);
214+
return true;
215+
});
200216
(void)foundArgs;
201217

202218
// Retains begin a lifetime scope, so we infer scan forwards.
203-
auto remark = RemarkMissed("memory-management", *rvi,
219+
auto remark = RemarkMissed("memory", *rvi,
204220
SourceLocInferenceBehavior::ForwardScanOnly)
205-
<< "Found retain:";
206-
if (inferredArgs.empty()) {
207-
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
208-
"Unable to infer any values being retained.");
209-
} else {
210-
for (auto arg : inferredArgs) {
211-
remark << arg;
212-
}
221+
<< "retain";
222+
for (auto arg : inferredArgs) {
223+
remark << arg;
213224
}
214225
return remark;
215226
});
@@ -221,26 +232,20 @@ void OptRemarkGeneratorInstructionVisitor::visitReleaseValueInst(
221232
using namespace OptRemark;
222233
SILValue root = rcfi.getRCIdentityRoot(rvi->getOperand());
223234
SmallVector<Argument, 8> inferredArgs;
224-
bool foundArgs = valueToDeclInferrer.infer(
225-
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
226-
inferredArgs.push_back(arg);
227-
return true;
228-
});
235+
bool foundArgs = valueToDeclInferrer.infer(ArgumentKeyKind::Note, root,
236+
[&](Argument arg) {
237+
inferredArgs.push_back(arg);
238+
return true;
239+
});
229240
(void)foundArgs;
230241

231242
// Releases end a lifetime scope so we infer scan backward.
232-
auto remark = RemarkMissed("memory-management", *rvi,
243+
auto remark = RemarkMissed("memory", *rvi,
233244
SourceLocInferenceBehavior::BackwardScanOnly)
234-
<< "Found release:";
235-
if (inferredArgs.empty()) {
236-
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
237-
"Unable to infer any values being released.");
238-
} else {
239-
for (auto arg : inferredArgs) {
240-
remark << arg;
241-
}
245+
<< "release";
246+
for (auto arg : inferredArgs) {
247+
remark << arg;
242248
}
243-
244249
return remark;
245250
});
246251
}

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,10 @@ import Builtin
66

77
sil @foo : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
88
bb0(%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.}}
9+
strong_retain %0 : $Builtin.NativeObject // expected-remark {{retain}}
10+
retain_value %0 : $Builtin.NativeObject // expected-remark {{retain}}
11+
strong_release %0 : $Builtin.NativeObject // expected-remark {{release}}
12+
release_value %0 : $Builtin.NativeObject // expected-remark {{release}}
1713
%9999 = tuple()
1814
return %9999 : $()
1915
}

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

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,47 +5,47 @@
55

66
// CHECK: --- !Missed
77
// CHECK-NEXT: Pass: sil-opt-remark-gen
8-
// CHECK-NEXT: Name: sil.memory-management
8+
// CHECK-NEXT: Name: sil.memory
99
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
1010
// CHECK-NEXT: Line: 59, Column: 5 }
1111
// CHECK-NEXT: Function: 'getGlobal()'
1212
// CHECK-NEXT: Args:
13-
// CHECK-NEXT: - String: 'Found retain:'
14-
// CHECK-NEXT: - InferredValue: 'on value:'
13+
// CHECK-NEXT: - String: retain
14+
// CHECK-NEXT: - InferredValue: 'of ''global'''
1515
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
1616
// CHECK-NEXT: Line: 55, Column: 12 }
1717
// CHECK-NEXT: ...
1818
// CHECK-NEXT: --- !Missed
1919
// CHECK-NEXT: Pass: sil-opt-remark-gen
20-
// CHECK-NEXT: Name: sil.memory-management
20+
// CHECK-NEXT: Name: sil.memory
2121
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
2222
// CHECK-NEXT: Line: 67, Column: 5 }
2323
// CHECK-NEXT: Function: 'useGlobal()'
2424
// CHECK-NEXT: Args:
25-
// CHECK-NEXT: - String: 'Found retain:'
26-
// CHECK-NEXT: - InferredValue: 'on value:'
25+
// CHECK-NEXT: - String: retain
26+
// CHECK-NEXT: - InferredValue: 'of ''x'''
2727
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
2828
// CHECK-NEXT: Line: 64, Column: 9 }
2929
// CHECK-NEXT: ...
3030
// CHECK-NEXT: --- !Missed
3131
// CHECK-NEXT: Pass: sil-opt-remark-gen
32-
// CHECK-NEXT: Name: sil.memory-management
32+
// CHECK-NEXT: Name: sil.memory
3333
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
3434
// CHECK-NEXT: Line: 67, Column: 12 }
3535
// CHECK-NEXT: Function: 'useGlobal()'
3636
// CHECK-NEXT: Args:
37-
// CHECK-NEXT: - String: 'Found release:'
38-
// CHECK-NEXT: - InferValueFailure: Unable to infer any values being released.
37+
// CHECK-NEXT: - String: release
38+
3939
// CHECK-NEXT: ...
4040
// CHECK-NEXT: --- !Missed
4141
// CHECK-NEXT: Pass: sil-opt-remark-gen
42-
// CHECK-NEXT: Name: sil.memory-management
42+
// CHECK-NEXT: Name: sil.memory
4343
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
4444
// CHECK-NEXT: Line: 67, Column: 12 }
4545
// CHECK-NEXT: Function: 'useGlobal()'
4646
// CHECK-NEXT: Args:
47-
// CHECK-NEXT: - String: 'Found release:'
48-
// CHECK-NEXT: - InferredValue: 'on value:'
47+
// CHECK-NEXT: - String: release
48+
// CHECK-NEXT: - InferredValue: 'of ''x'''
4949
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
5050
// CHECK-NEXT: Line: 64, Column: 9 }
5151
// CHECK-NEXT: ...
@@ -56,18 +56,17 @@ public var global = Klass()
5656

5757
@inline(never)
5858
public func getGlobal() -> Klass {
59-
return global // expected-remark @:5 {{Found retain:}}
60-
// expected-note @-5:12 {{on value:}}
59+
return global // expected-remark @:5 {{retain}}
60+
// expected-note @-5:12 {{of 'global'}}
6161
}
6262

6363
public func useGlobal() {
6464
let x = getGlobal()
6565
// Make sure that the retain msg is at the beginning of the print and the
6666
// releases are the end of the print.
67-
print(x) // expected-remark @:5 {{Found retain:}}
68-
// expected-note @-4:9 {{on value:}}
69-
// expected-remark @-2:12 {{Found release:}}
70-
// expected-note @-3:12 {{Unable to infer any values being released.}}
71-
// expected-remark @-4:12 {{Found release:}}
72-
// expected-note @-8:9 {{on value:}}
67+
print(x) // expected-remark @:5 {{retain}}
68+
// expected-note @-4:9 {{of 'x'}}
69+
// expected-remark @-2:12 {{release}}
70+
// expected-remark @-3:12 {{release}}
71+
// expected-note @-7:9 {{of 'x'}}
7372
}

0 commit comments

Comments
 (0)