[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.
This commit is contained in:
Michael Gottesman
2020-07-20 19:35:48 -07:00
parent aea6d7df80
commit 2c9a34fa01
5 changed files with 173 additions and 45 deletions

View File

@@ -21,6 +21,7 @@
#include "swift/Basic/SourceLoc.h"
#include "swift/Demangling/Demangler.h"
#include "swift/SIL/SILArgument.h"
#include "swift/SIL/SILBasicBlock.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SIL/SILModule.h"
@@ -95,11 +96,12 @@ struct Argument {
/// If set, the debug location corresponding to the value.
SourceLoc loc;
explicit Argument(StringRef Str = "")
: key(ArgumentKeyKind::Default, "String"), val(Str) {}
Argument(StringRef key, StringRef val)
: key(ArgumentKeyKind::Default, key), val(val) {}
explicit Argument(StringRef str = "")
: Argument({ArgumentKeyKind::Default, "String"}, str) {}
Argument(StringRef key, StringRef val)
: Argument({ArgumentKeyKind::Default, key}, val) {}
Argument(ArgumentKey key, StringRef val) : key(key), val(val) {}
Argument(StringRef key, int n);
Argument(StringRef key, long n);
Argument(StringRef key, long long n);
@@ -112,6 +114,24 @@ struct Argument {
Argument(ArgumentKey key, SILFunction *f);
Argument(StringRef key, SILType ty);
Argument(StringRef key, CanType ty);
Argument(StringRef key, StringRef msg, const ValueDecl *decl)
: Argument(ArgumentKey(ArgumentKeyKind::Default, key), msg, decl) {}
Argument(ArgumentKey key, StringRef msg, const ValueDecl *decl)
: key(key), val(msg), loc(decl->getLoc()) {}
/// Given a value, call \p funcPassedInferredArgs for each associated
/// ValueDecl that is associated with \p value. All created Arguments are
/// passed the same StringRef. To stop iteration, return false in \p
/// funcPassedInferedArgs.
///
/// NOTE: the function may be called multiple times if the optimizer joined
/// two live ranges and thus when iterating over value's users we see multiple
/// debug_value operations.
static bool
inferArgumentsForValue(ArgumentKeyKind keyKind, StringRef message,
SILValue value,
function_ref<bool(Argument)> funcPassedInferedArgs);
};
/// Shorthand to insert named-value pairs.

View File

@@ -20,6 +20,9 @@
#include "swift/AST/DiagnosticEngine.h"
#include "swift/AST/DiagnosticsSIL.h"
#include "swift/Demangling/Demangler.h"
#include "swift/SIL/DebugUtils.h"
#include "swift/SIL/SILArgument.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SIL/SILRemarkStreamer.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/CommandLine.h"
@@ -72,6 +75,28 @@ Argument::Argument(StringRef key, CanType ty)
ty.print(stream);
}
bool Argument::inferArgumentsForValue(
ArgumentKeyKind keyKind, StringRef msg, SILValue value,
function_ref<bool(Argument)> funcPassedInferedArgs) {
// If we have an argument, just use that.
if (auto *arg = dyn_cast<SILArgument>(value))
if (auto *decl = arg->getDecl())
return funcPassedInferedArgs(
Argument({keyKind, "InferredValue"}, msg, decl));
// TODO: Look for loads from globals and addresses.
// Otherwise, look for debug_values.
for (auto *use : getDebugUses(value))
if (auto *dvi = dyn_cast<DebugValueInst>(use->getUser()))
if (auto *decl = dvi->getDecl())
if (!funcPassedInferedArgs(
Argument({keyKind, "InferredValue"}, msg, decl)))
return false;
return true;
}
template <typename DerivedT>
std::string Remark<DerivedT>::getMsg() const {
std::string str;

View File

@@ -17,6 +17,7 @@
#include "swift/SIL/SILInstruction.h"
#include "swift/SIL/SILModule.h"
#include "swift/SIL/SILVisitor.h"
#include "swift/SILOptimizer/Analysis/RCIdentityAnalysis.h"
#include "swift/SILOptimizer/PassManager/Passes.h"
#include "swift/SILOptimizer/PassManager/Transforms.h"
@@ -31,10 +32,12 @@ namespace {
struct OptRemarkGeneratorInstructionVisitor
: public SILInstructionVisitor<OptRemarkGeneratorInstructionVisitor> {
SILModule &mod;
RCIdentityFunctionInfo &rcfi;
OptRemark::Emitter ORE;
OptRemarkGeneratorInstructionVisitor(SILModule &mod)
: mod(mod), ORE(DEBUG_TYPE, mod) {}
OptRemarkGeneratorInstructionVisitor(SILModule &mod,
RCIdentityFunctionInfo &rcfi)
: mod(mod), rcfi(rcfi), ORE(DEBUG_TYPE, mod) {}
void visitStrongRetainInst(StrongRetainInst *sri);
void visitStrongReleaseInst(StrongReleaseInst *sri);
@@ -49,10 +52,28 @@ void OptRemarkGeneratorInstructionVisitor::visitStrongRetainInst(
StrongRetainInst *sri) {
ORE.emit([&]() {
using namespace OptRemark;
SILValue root = rcfi.getRCIdentityRoot(sri->getOperand());
SmallVector<Argument, 8> inferredArgs;
bool foundArgs = Argument::inferArgumentsForValue(
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
inferredArgs.push_back(arg);
return true;
});
(void)foundArgs;
// Retains begin a lifetime scope so we infer scan forward.
return RemarkMissed("memory-management", *sri,
auto remark = RemarkMissed("memory-management", *sri,
SourceLocInferenceBehavior::ForwardScanOnly)
<< "Unable to remove retain";
<< "Found retain:";
if (inferredArgs.empty()) {
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
"Unable to infer any values being retained.");
} else {
for (auto arg : inferredArgs) {
remark << arg;
}
}
return remark;
});
}
@@ -61,9 +82,26 @@ void OptRemarkGeneratorInstructionVisitor::visitStrongReleaseInst(
ORE.emit([&]() {
using namespace OptRemark;
// Releases end a lifetime scope so we infer scan backward.
return RemarkMissed("memory-management", *sri,
SILValue root = rcfi.getRCIdentityRoot(sri->getOperand());
SmallVector<Argument, 8> inferredArgs;
bool foundArgs = Argument::inferArgumentsForValue(
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
inferredArgs.push_back(arg);
return true;
});
(void)foundArgs;
auto remark = RemarkMissed("memory-management", *sri,
SourceLocInferenceBehavior::BackwardScanOnly)
<< "Unable to remove release";
<< "Found release:";
if (inferredArgs.empty()) {
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
"Unable to infer any values being released.");
} else {
for (auto arg : inferredArgs) {
remark << arg;
}
}
return remark;
});
}
@@ -71,10 +109,28 @@ void OptRemarkGeneratorInstructionVisitor::visitRetainValueInst(
RetainValueInst *rvi) {
ORE.emit([&]() {
using namespace OptRemark;
SILValue root = rcfi.getRCIdentityRoot(rvi->getOperand());
SmallVector<Argument, 8> inferredArgs;
bool foundArgs = Argument::inferArgumentsForValue(
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
inferredArgs.push_back(arg);
return true;
});
(void)foundArgs;
// Retains begin a lifetime scope, so we infer scan forwards.
return RemarkMissed("memory-management", *rvi,
auto remark = RemarkMissed("memory-management", *rvi,
SourceLocInferenceBehavior::ForwardScanOnly)
<< "Unable to remove retain";
<< "Found retain:";
if (inferredArgs.empty()) {
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
"Unable to infer any values being retained.");
} else {
for (auto arg : inferredArgs) {
remark << arg;
}
}
return remark;
});
}
@@ -82,10 +138,29 @@ void OptRemarkGeneratorInstructionVisitor::visitReleaseValueInst(
ReleaseValueInst *rvi) {
ORE.emit([&]() {
using namespace OptRemark;
SILValue root = rcfi.getRCIdentityRoot(rvi->getOperand());
SmallVector<Argument, 8> inferredArgs;
bool foundArgs = Argument::inferArgumentsForValue(
ArgumentKeyKind::Note, "on value:", root, [&](Argument arg) {
inferredArgs.push_back(arg);
return true;
});
(void)foundArgs;
// Releases end a lifetime scope so we infer scan backward.
return RemarkMissed("memory-management", *rvi,
auto remark = RemarkMissed("memory-management", *rvi,
SourceLocInferenceBehavior::BackwardScanOnly)
<< "Unable to remove release";
<< "Found release:";
if (inferredArgs.empty()) {
remark << Argument({ArgumentKeyKind::ParentLocNote, "InferValueFailure"},
"Unable to infer any values being released.");
} else {
for (auto arg : inferredArgs) {
remark << arg;
}
}
return remark;
});
}
@@ -115,7 +190,8 @@ class OptRemarkGenerator : public SILFunctionTransform {
auto *fn = getFunction();
LLVM_DEBUG(llvm::dbgs() << "Visiting: " << fn->getName() << "\n");
OptRemarkGeneratorInstructionVisitor visitor(fn->getModule());
auto &rcfi = *getAnalysis<RCIdentityAnalysis>()->get(fn);
OptRemarkGeneratorInstructionVisitor visitor(fn->getModule(), rcfi);
for (auto &block : *fn) {
for (auto &inst : block) {
visitor.visit(&inst);

View File

@@ -1,13 +1,4 @@
// RUN: %target-sil-opt -sil-opt-remark-generator -sil-remarks-missed=sil-opt-remark-gen %s -o /dev/null 2>&1 | %FileCheck %s
// CHECK: {{.*}}opt-remark-generator.sil:18:3: remark: Unable to remove retain
// CHECK-NEXT: strong_retain
// CHECK: {{.*}}opt-remark-generator.sil:19:3: remark: Unable to remove retain
// CHECK-NEXT: retain_value
// CHECK: {{.*}}opt-remark-generator.sil:20:3: remark: Unable to remove release
// CHECK-NEXT: strong_release
// CHECK: {{.*}}opt-remark-generator.sil:21:3: remark: Unable to remove release
// CHECK-NEXT: release_value
// RUN: %target-sil-opt -sil-opt-remark-generator -sil-remarks-missed=sil-opt-remark-gen -verify %s -o /dev/null
sil_stage canonical
@@ -15,10 +6,14 @@ import Builtin
sil @foo : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
bb0(%0 : $Builtin.NativeObject):
strong_retain %0 : $Builtin.NativeObject
retain_value %0 : $Builtin.NativeObject
strong_release %0 : $Builtin.NativeObject
release_value %0 : $Builtin.NativeObject
strong_retain %0 : $Builtin.NativeObject // expected-remark {{Found retain:}}
// expected-note @-1 {{Unable to infer any values being retained.}}
retain_value %0 : $Builtin.NativeObject // expected-remark {{Found retain:}}
// expected-note @-1 {{Unable to infer any values being retained.}}
strong_release %0 : $Builtin.NativeObject // expected-remark {{Found release:}}
// expected-note @-1 {{Unable to infer any values being released.}}
release_value %0 : $Builtin.NativeObject // expected-remark {{Found release:}}
// expected-note @-1 {{Unable to infer any values being released.}}
%9999 = tuple()
return %9999 : $()
}

View File

@@ -7,37 +7,45 @@
// CHECK-NEXT: Pass: sil-opt-remark-gen
// CHECK-NEXT: Name: sil.memory-management
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
// CHECK-NEXT: Line: 49, Column: 5 }
// CHECK-NEXT: Line: 57, Column: 5 }
// CHECK-NEXT: Function: 'getGlobal()'
// CHECK-NEXT: Args:
// CHECK-NEXT: - String: Unable to remove retain
// CHECK-NEXT: - String: 'Found retain:'
// CHECK-NEXT: - InferValueFailure: Unable to infer any values being retained.
// CHECK-NEXT: ...
// CHECK-NEXT: --- !Missed
// CHECK-NEXT: Pass: sil-opt-remark-gen
// CHECK-NEXT: Name: sil.memory-management
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
// CHECK-NEXT: Line: 56, Column: 5 }
// CHECK-NEXT: Line: 65, Column: 5 }
// CHECK-NEXT: Function: 'useGlobal()'
// CHECK-NEXT: Args:
// CHECK-NEXT: - String: Unable to remove retain
// CHECK-NEXT: - String: 'Found retain:'
// CHECK-NEXT: - InferredValue: 'on value:'
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
// CHECK-NEXT: Line: 62, Column: 9 }
// CHECK-NEXT: ...
// CHECK-NEXT: --- !Missed
// CHECK-NEXT: Pass: sil-opt-remark-gen
// CHECK-NEXT: Name: sil.memory-management
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
// CHECK-NEXT: Line: 56, Column: 12 }
// CHECK-NEXT: Line: 65, Column: 12 }
// CHECK-NEXT: Function: 'useGlobal()'
// CHECK-NEXT: Args:
// CHECK-NEXT: - String: Unable to remove release
// CHECK-NEXT: - String: 'Found release:'
// CHECK-NEXT: - InferValueFailure: Unable to infer any values being released.
// CHECK-NEXT: ...
// CHECK-NEXT: --- !Missed
// CHECK-NEXT: Pass: sil-opt-remark-gen
// CHECK-NEXT: Name: sil.memory-management
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
// CHECK-NEXT: Line: 56, Column: 12 }
// CHECK-NEXT: Line: 65, Column: 12 }
// CHECK-NEXT: Function: 'useGlobal()'
// CHECK-NEXT: Args:
// CHECK-NEXT: - String: Unable to remove release
// CHECK-NEXT: - String: 'Found release:'
// CHECK-NEXT: - InferredValue: 'on value:'
// CHECK-NEXT: DebugLoc: { File: '{{.*}}opt-remark-generator.swift',
// CHECK-NEXT: Line: 62, Column: 9 }
// CHECK-NEXT: ...
public class Klass {}
@@ -46,14 +54,18 @@ public var global = Klass()
@inline(never)
public func getGlobal() -> Klass {
return global // expected-remark @:5 {{Unable to remove retain}}
return global // expected-remark @:5 {{Found retain:}}
// expected-note @-1:5 {{Unable to infer any values being retained.}}
}
public func useGlobal() {
let x = getGlobal()
// Make sure that the retain msg is at the beginning of the print and the
// releases are the end of the print.
print(x) // expected-remark @:5 {{Unable to remove retain}}
// expected-remark @-1:12 {{Unable to remove release}}
// expected-remark @-2:12 {{Unable to remove release}}
print(x) // expected-remark @:5 {{Found retain:}}
// expected-note @-4:9 {{on value:}}
// expected-remark @-2:12 {{Found release:}}
// expected-note @-3:12 {{Unable to infer any values being released.}}
// expected-remark @-4:12 {{Found release:}}
// expected-note @-8:9 {{on value:}}
}