[PATCH] D104129: [CSSPGO] Report zero-count probe in profile instead of dangling probes.

Mon Jun 14 12:59:27 PDT 2021

hoy added a comment.

In D104129#2817282 <https://reviews.llvm.org/D104129#2817282>, @spupyrev wrote:

> It is strange that representation of probes affects counts quality (and hence, generated binaries). Is this a temporary state (that hopefully will be improved in the future)?
>
> In general I agree that the suggested representation (0 for zero-counts, missing for dangles) is way more readable, and thus, less-error-prone.

The current change affects counts quality in that

1. Reporting more dangling probes than real dead probes. I believe we haven't spotted all places in the compiler that can remove dangling probes, and in those cases dangle probes were treated as real dead. With the current patch, more probes including some real dead probes are reported as dangling. This could be good or bad, depending on benchmarks. Current results show this is better in general. We're thinking about identifying real dead probes in the compiler and specially mark them as the next step.

2. Reporting what's already collected on the non-dangling sibling of a dangling probe. Previously reporting the danglingness of a probe caused real samples collected on other non-dangling copies of the same probe to be lost.

#2 can be improved by reporting both danglingness and real samples in future, if that's helpful We would need to figure out how to encode them together.

I collected more data for this change. For SPEC2017, besides the counts quality gain, I've also seen a 0.4% perf gain on average, mainly from 602.gcc_s, 623.xalancbmk_s and 625.x264_s. For cinder, a 8% counts quality gain with a neutral perf result.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104129/new/

https://reviews.llvm.org/D104129