Stream: t-compiler/wg-profile-guided-optimization

Topic: code coverage


Philipp Hansch (Apr 18 2019 at 07:52, on Zulip):

heya, just curious. Is profile guided optimization a prerequisite for instrumenting LLVM to get coverage information for rustc code?

Philipp Hansch (Apr 18 2019 at 07:53, on Zulip):

I found this comment which seems to say so

mw (Apr 18 2019 at 09:43, on Zulip):

I think this is a different kind of instrumentation (although it records similar data)

mw (Apr 18 2019 at 09:44, on Zulip):

although that comment looks like it does something that actually builds on PGO instrumentation (instead of GCOV)

bebytesback (May 07 2019 at 02:28, on Zulip):

Hi @mw ,
Have there been any discussion about making rustc insert the PGO instrumentation that @bossmc mention in the above comment?

mw (May 07 2019 at 08:45, on Zulip):

there are two ways of adding this kind of instrumentation to LLVM IR:

Both use the same LLVM intrinsics but the first one relies on the compiler front-end (e.g. clang or rustc) to insert them, while the second one lets LLVM automatically insert them during the optimization passes.

mw (May 07 2019 at 08:46, on Zulip):

Right now, rustc only supports the IR-level approach and (afaik) there are no immediate plans to support the front-end approach.

mw (May 07 2019 at 08:48, on Zulip):

I don't know the details about this code coverage technique but from taking a quick look at the documentation, it only talks about front-end based instrumentation. not sure if it would also work with IR-level instrumentation.

mw (May 07 2019 at 08:54, on Zulip):

the project this working group is about is focused on make PGO work though, code coverage would have to be a separate effort

bebytesback (May 07 2019 at 16:43, on Zulip):

Thanks, I understand. Would the working group be interested in looking into the feasibility in adding Front-end based instrumentation?

mw (May 10 2019 at 13:02, on Zulip):

since I am currently the only member of this working "group" and I'll be going on parental leave next month, I don't think it's likely that this will happen. I personally would also like to avoid adding front-end based instrumentation if possible (it's another feature that needs to be maintained).

mw (May 10 2019 at 13:02, on Zulip):

I would suggest trying to find out if IR-level instrumentation would also work here.

mw (May 10 2019 at 13:03, on Zulip):

the corresponding clang commandline option is -fprofile-generate

mw (May 10 2019 at 13:03, on Zulip):

(while FE-based instrumentation is enabled in clang via -fprofile-instr-generate)

Gary Tierney (Oct 23 2019 at 19:31, on Zulip):

Hi, I've been taking a look at getting a prototype going for the front-end based approach, but don't quite know my way around the librustc_codegen_* architecture. Could anyone direct me towards where I could start emitting additional IR for e.g. a Function?

The goal is to emit the LLVM intrinsics mentioned by @mw above, just a single counter for function entry as a starter. My current idea is to add something like emit_profiling_intrinsic(...) to Builder in librustc_codegen_ssa and implement in librustc_codegen_llvm. Then I can start injecting profiling intrinsics from existing functions like codegen_statement in the SSA crate. Is that reasonable?

I did look at using the existing IR-level approach for this, but don't see any way to reconcile the counters inserted into the IR by LLVM with the counters referenced in the coverage mapping.

mw (Oct 24 2019 at 07:59, on Zulip):

It's probably best if you re-post this message in the regular #t-compiler stream for better visibility.

gnzlbg (Oct 24 2019 at 09:14, on Zulip):

cc @eddyb

gnzlbg (Oct 24 2019 at 09:14, on Zulip):

I kind of expected to be around here: https://github.com/rust-lang/rust/blob/master/src/librustc_codegen_llvm/declare.rs#L133

gnzlbg (Oct 24 2019 at 09:16, on Zulip):

but that only declares fns with the intention to define it

gnzlbg (Oct 24 2019 at 09:20, on Zulip):

Maybe in the new_block ?

gnzlbg (Oct 24 2019 at 09:21, on Zulip):

https://github.com/rust-lang/rust/blob/e369d87b015a84653343032833d65d0545fd3f26/src/librustc_codegen_ssa/mir/mod.rs#L197

gnzlbg (Oct 24 2019 at 09:21, on Zulip):

You could add it to the "start" block of a function, or maybe earlier

Gary Tierney (Oct 24 2019 at 09:23, on Zulip):

@gnzlbg ah, thanks. I think codegen_mir(...) has the bulk of what I'm looking for. This should get me started on emitting the intrinsics.

gnzlbg (Oct 24 2019 at 09:24, on Zulip):

I think @eddyb or @bjorn3 are familiar with that code, so it might make sense to ping them if you have questions

gnzlbg (Oct 24 2019 at 09:25, on Zulip):

The birds eye is that librustc_codegen_ssa is the "API" for generating code for a backend from MIR

gnzlbg (Oct 24 2019 at 09:25, on Zulip):

and that the different backends (e.g. librustc_codegen_llvm, librustc_codegen_clif, etc.) implement that API

gnzlbg (Oct 24 2019 at 09:26, on Zulip):

You might get away touching only the LLVM backend (e.g. for the "start" label, also insert some code coverage code)

gnzlbg (Oct 24 2019 at 09:27, on Zulip):

but I can't tell you whether that's a good idea

Gary Tierney (Oct 24 2019 at 09:31, on Zulip):

Thanks, great info. I think I'll start with adding something like a newProfilingInstrumentationMethods to librustc_codegen_ssa that I can implement for the LLVM backend. That'll let me insert counters generically from the "API" crate and the LLVM backend can look at these counters and associate it with coverage maps. I can start stripping it back later if it living in librustc_codegen_ssa turns out to be a bad idea.

eddyb (Oct 24 2019 at 16:46, on Zulip):

@Gary Tierney hey! so I did a bit of research last year about this for a contract that never happened, and I think I might remember a few things

eddyb (Oct 24 2019 at 16:46, on Zulip):

my suggestions is not adding another trait but just methods to BuilderMethods or w/e

eddyb (Oct 24 2019 at 16:46, on Zulip):

specifically for inserting runtime calls, at least

Gary Tierney (Oct 24 2019 at 17:16, on Zulip):

Hi @eddyb :)

Yep, that seems reasonable. I don't quite get the delineation between the various traits at the moment, I'd just found DebugInfoBuilderMethods and started copying from that ;).

My intention at the moment is to have an API for inserting runtime calls in BuilderMethods, et al. and leave it up to the LLVM backend to map source spans to profiling counters when it begins emitting a function or compiling a module.

eddyb (Oct 24 2019 at 17:17, on Zulip):

FooBuilderMethods is a bit of an anti-pattern, and debuginfo is kinda special, so I'd avoid mimicking it

Gary Tierney (Oct 24 2019 at 17:26, on Zulip):

Noted, I'll just add some simple functions to BuilderMethods for now.. Getting the backend agnostic code to start emitting the calls doesn't
seem too difficult.

I'll report any progress on https://github.com/rust-lang/rust/issues/34701.

Last update: Nov 15 2019 at 11:00UTC