Skip to content
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.

Sign up for GitHub

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

Jump to bottom

InstrumentCoverage requires information from HIR that might need to be available via MIR #79625

Open
richkadel opened this issue Dec 2, 2020 · 1 comment
Open

InstrumentCoverage requires information from HIR that might need to be available via MIR #79625

richkadel opened this issue Dec 2, 2020 · 1 comment
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) A-HIR Area: The high-level intermediate representation (HIR) A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@richkadel
Copy link
Contributor

Coverage requires the spans for the FnSig and hir::Body.

It also uses the hir::Body to compute a function hash, required by LLVM intrinsic instrprof.coverage.

According to this code review comment:

#79109 (comment)

MIR strives to be a self-contained IR which means that operations on it should only use the information in MIR

Consider improving MIR to provide the information needed, to avoid going back to HIR for it.
@richkadel richkadel mentioned this issue Dec 2, 2020
Merged
@oli-obk oli-obk added A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) A-HIR Area: The high-level intermediate representation (HIR) A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html labels Dec 2, 2020
@richkadel
Copy link
Contributor Author

richkadel commented Dec 9, 2020 •
edited
Loading

Just noting for clarification: There are about a dozen cases where HIR data is accessed to generate the MIR (based on a simple regex search hir.*Body), most of which are not related to coverage.

I'm not sure that the A-code-coverage label is necessary, but, regardless, IMO, this issue should not impact stabilization of Rust Coverage.

cc: @tmandry @wesleywiser

@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@Zalathar Zalathar mentioned this issue Nov 24, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) A-HIR Area: The high-level intermediate representation (HIR) A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

coverage: Collect HIR info during MIR building Zalathar/rust
3 participants
@oli-obk @richkadel @Noratrieb