LGTM, thanks!

In D104129#2822242, @hoy wrote:

In D104129#2821245, @wlei wrote:

Curious why we can remove the moveAndDanglePseudoProbes(don't need the concept of dangling)? My understanding is we treat dangling as unknown, at pass1 we need to pass this information to the profiled binary. An optimized-out probe won't have samples, it will be treated as zero if not marked as dangling.

We now use "missing" to represent dangling which is natural, and "missing" is different from zero. The counts inferencer will reason about counts for missing probes. We will flip the concept of dangling to real dead, and likely make a routine like moveAndKillPseudoProbes.

Curious why we can remove the moveAndDanglePseudoProbes(don't need the concept of dangling)? My understanding is we treat dangling as unknown, at pass1 we need to pass this information to the profiled binary. An optimized-out probe won't have samples, it will be treated as zero if not marked as dangling.

Thanks for the fix!

Addressing Wenlei's comments

change to use uint32_t

Addressing Wenlei's comments

Thanks for working on this, LGTM!

LGTM, thanks!

LGTM, thanks for adding useful debug printer.

In D102721#2767154, @hoy wrote:

Thanks for the fix. Is it possible to add a test?

Address Hongtao's feedback, add test case

In D102429#2760386, @hoy wrote:

In D102429#2760347, @wlei wrote:

Try to understand this:

So we merge the samples from the same function but different context, does this cause the issue?

Exactly.

Such probes should not share samples and their factors should be fixed up separately.

So with that, when code duplication happens, we need to fix probe factor, so here it will do it separately by grouping the probe by different inline context. then each group work for one context inlining. is my understanding correct?

That's right. Probe in a function profile can have multiple copies due to code duplication. We just want to share the corresponding samples among these duplications, but not among all probes with same id but from different function profiles.

Try to understand this:

Update the comments according reviewers' discussion on profile-accurate. Currently, this version only work for probe-based code, it won't be affected by the source drift(see the comments)

update comments

address Hongtao's feedback: refine the test case

fix assertion condition

addressing Wenlei's feedback

LGTM, thanks for the fix.

switch to explicit cast

Addressing Hongtao's feedback

ad test case: manually make a < 1 factor of the promotable call site, after ICP check the discriminator

lgtm, thanks!

LGTM, thanks!

Addressing Wei's feedback

Add test cases

In D98226#2612949, @wenlei wrote:

Looks like this is the only place where canonicalization is missing. Would be good to have a test case for proper profile name canonicalization (removing other suffix, but not unique), otherwise looks good.

LGTM.

use merge function to update body samples for probe

address reviewers' feedback

Thanks for working on this! LGTM

remove count towards total sample for dangling probes

change to extern cl::opt<bool> ShowDisassemblyOnly

address Wenlei and Hongtao's feedback: switch to use global command line variable and some refactoring work

fix clang-tidy

address reviewers' feedback:
rename showAssembly to show-disassembly-only and change back to print to stdout

Address Wenlei's feedback and fix lint

Address Hongtao's feedback: added test case for invalid script check

address Hongtao's feedback: added comments