Skip to content

Commit db845eb

Browse files
committed
Correct the ownership model for sil-opt
Destroying the SIL remark streamer after transferring ownership to LLVM is frought. For one, the streamer holds the remark file's stream open. Destroying it early doesn't accomodate sil-opt, which transfers control to LLVM before running passes that emit remarks. Instead, just take a reference to the context that the streamer gets parented onto. If the remarks streamer infrastructure could just hold the file stream open for us, we wouldn't have to do any of this.
1 parent 90d05c1 commit db845eb

File tree

5 files changed

+29
-7
lines changed

5 files changed

+29
-7
lines changed

include/swift/SIL/SILModule.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,9 +523,7 @@ class SILModule {
523523
swift::SILRemarkStreamer *getSILRemarkStreamer() {
524524
return silRemarkStreamer.get();
525525
}
526-
std::unique_ptr<swift::SILRemarkStreamer> takeSILRemarkStreamer() {
527-
return std::move(silRemarkStreamer);
528-
}
526+
529527
void installSILRemarkStreamer();
530528

531529
// This is currently limited to VarDecl because the visibility of global

include/swift/SIL/SILRemarkStreamer.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,19 @@ namespace swift {
2626

2727
class SILRemarkStreamer {
2828
private:
29+
enum class Owner {
30+
SILModule,
31+
LLVM,
32+
} owner;
33+
2934
/// The underlying LLVM streamer.
3035
///
3136
/// If owned by a SILModule, this will be non-null.
3237
std::unique_ptr<llvm::remarks::RemarkStreamer> streamer;
38+
/// The owning LLVM context.
39+
///
40+
/// If owned by LLVM, this will be non-null.
41+
llvm::LLVMContext *context;
3342

3443
/// The remark output stream used to record SIL remarks to a file.
3544
std::unique_ptr<llvm::raw_fd_ostream> remarkStream;

lib/IRGen/IRGen.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -911,8 +911,8 @@ static void initLLVMModule(const IRGenModule &IGM, SILModule &SIL) {
911911
"standard-library"),
912912
llvm::ConstantAsMetadata::get(Value)}));
913913

914-
if (auto streamer = SIL.takeSILRemarkStreamer()) {
915-
std::move(streamer)->intoLLVMContext(Module->getContext());
914+
if (auto *streamer = SIL.getSILRemarkStreamer()) {
915+
streamer->intoLLVMContext(Module->getContext());
916916
}
917917
}
918918

lib/SIL/Utils/SILRemarkStreamer.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,35 @@ using namespace swift;
1919
SILRemarkStreamer::SILRemarkStreamer(
2020
std::unique_ptr<llvm::remarks::RemarkStreamer> &&streamer,
2121
std::unique_ptr<llvm::raw_fd_ostream> &&stream, const ASTContext &Ctx)
22-
: streamer(std::move(streamer)),
22+
: owner(Owner::SILModule), streamer(std::move(streamer)), context(nullptr),
2323
remarkStream(std::move(stream)), ctx(Ctx) { }
2424

2525
llvm::remarks::RemarkStreamer &SILRemarkStreamer::getLLVMStreamer() {
26+
switch (owner) {
27+
case Owner::SILModule:
28+
return *streamer.get();
29+
case Owner::LLVM:
30+
return *context->getMainRemarkStreamer();
31+
}
2632
return *streamer.get();
2733
}
2834

2935
const llvm::remarks::RemarkStreamer &
3036
SILRemarkStreamer::getLLVMStreamer() const {
37+
switch (owner) {
38+
case Owner::SILModule:
39+
return *streamer.get();
40+
case Owner::LLVM:
41+
return *context->getMainRemarkStreamer();
42+
}
3143
return *streamer.get();
3244
}
3345

3446
void SILRemarkStreamer::intoLLVMContext(llvm::LLVMContext &Ctx) & {
47+
assert(owner == Owner::SILModule);
3548
Ctx.setMainRemarkStreamer(std::move(streamer));
49+
context = &Ctx;
50+
owner = Owner::LLVM;
3651
}
3752

3853
std::unique_ptr<SILRemarkStreamer>

tools/sil-opt/SILOpt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ int main(int argc, char **argv) {
461461
if (CI.getSILModule())
462462
CI.getSILModule()->setSerializeSILAction([]{});
463463

464-
if (RemarksFilename != "") {
464+
if (!RemarksFilename.empty()) {
465465
llvm::Expected<llvm::remarks::Format> formatOrErr =
466466
llvm::remarks::parseFormat(RemarksFormat);
467467
if (llvm::Error E = formatOrErr.takeError()) {

0 commit comments

Comments
 (0)