Skip to content

[5.3] Lazily Associate a SILRemarkStreamer with an LLVMContext #31221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 23, 2020

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Apr 22, 2020

Cherry-pick #31167 to 5.3 because the refactoring that broke this went in before the branch date.

CodaFi added 2 commits April 22, 2020 16:15
Corrects a mistake introduced in swiftlang#31106

I was under the impression that the LLVMContext for an instance of
llvm::remarks::RemarkStreamer was somehow just scratch-space. It turns
out the ASMPrinters don't gather remarks data from modules, they gather
it from the remark streamer associated with the module's context.

Thus, we cannot have the module's context be distinct from whatever
context the streamer is eventually associated with.

In order to bring these two systems back into harmony, introduce a simple
ownership contract to SILRemarkStreamer. That is, it starts owned by
a SILModule, in which case the SILRemarkStreamer holds onto the
underlying LLVM object as the optimizer emits remarks. When it comes
time to IRGen the module, then and only then do we install the streamer
on the module's context. This tranfers ownership of the underlying LLVM
streamer to LLVM itself, so it acts as a consuming operation. When we
are about to perform IR Generation, the SILModule will be expiring
anyways, so the streamer was already about to be destroyed.

We're just putting it to better use doing its job.
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.
@CodaFi CodaFi requested a review from francisvm April 22, 2020 23:17
@CodaFi
Copy link
Contributor Author

CodaFi commented Apr 22, 2020

@swift-ci test and merge

@compnerd compnerd added the r5.3 label Apr 22, 2020
@CodaFi
Copy link
Contributor Author

CodaFi commented Apr 23, 2020

Builder died

@swift-ci test macOS

@CodaFi
Copy link
Contributor Author

CodaFi commented Apr 23, 2020

@CodaFi CodaFi merged commit 6921723 into swiftlang:release/5.3 Apr 23, 2020
@CodaFi CodaFi deleted the streamer-redeemer branch April 23, 2020 04:20
@AnthonyLatsis AnthonyLatsis added swift 5.3 🍒 release cherry pick Flag: Release branch cherry picks labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants