[PATCH] D124490: [InstrProf] Minimal Block Coverage

[Ellis Hoag via Phabricator via llvm-commits]

ellis added a comment.

In D124490#3520905 <https://reviews.llvm.org/D124490#3520905>, @gulfem wrote:

> Hi @ellis,
>
> This is exciting for us! We are interested in using single byte booleans for `source-based code coverage`, and I am currently working on the implementation.
> We are trying to find a good solution to the problem that you described that missing counts cannot be inferred by doing arithmetic on other block counts when we use boolean counters.
> For ex, in an `if` statement, `else` part is not instrumented, and its counter is inferred by subtracting `parent` and `then` counters.
> When we use the boolean values for counters, this cannot be done anymore.
> Simpler approach is to instrument more basic blocks, but we are exploring approaches that add counters to minimal blocks.
> One of the biggest differences of `coverage` and `PGO` is that counters are emitted by traversing `AST` nodes in the front-end for `coverage`, whereas they are emitted by traversing `CFG` nodes for `PGO`.
> The algorithm that you introduced is based on `CFG` traversal. I'm looking at that to see whether this can be repurposed for `coverage`.
>
> A few questions:
>
> 1. You mentioned that "we found that only ~60% of basic blocks need to be instrumented". With boolean counters, do you how much that has changed?
> 2. Size reduction is great! Do you have any data on the impact of block coverage on compilation time and runtime performance? For `coverage`, we are also aiming runtime performance with boolean counters.
> 3. Did you do any verification to compare the correctness of block coverage like comparing it against block counts?
>
> It seems like we are trying to solve similar problems in different contexts, so we would be very interested in collaboration.

I'm happy to see interest in coverage instrumentation!

1. Let me clarify. The existing "Kirchhoff circuit law" optimization used for 64 bit **counters** allows us to instrument a subset of basic blocks. IIRC we need to instrument slightly more than 60% of basic blocks to do this. For the minimal block **coverage** algorithm I've implemented here, we also only need to instrument ~60% of basic blocks. Of course, the blocks we instrument are not the same in both algorithms, but the number of blocks is roughly the same.
2. I haven't analyzed compilation time, but the theoretical runtime is O(|E| * |V|). For runtime performance, I also haven't measured this but my intuition is that this is very close to optimal since we are adding a single store in as few blocks as possible. In some cases there is some choice in which blocks to instrument and so we could try to make better decisions there, e.g., choose to instrument a rarely executed block rather than one in a tight loop.
3. My colleague Julian Mestre actually proved that the algorithm produces a correct instrumentation and also that it produces a minimal instrumentation. So we cannot find a coverage that instruments fewer blocks. We are planning on publishing these results in a paper, but that might not be for some time.

I've implemented `BlockCoverageInference` to assume we are instrumenting Blocks in a CFG, but there is nothing special about blocks. This algorithm will work on any directed graph with entry and exit nodes. I think we can extend this class to support a more general graph and use it for AST coverage. Let me know if you think this could work.

I'm also wondering if we could use this PGO coverage to infer source-based code coverage by looking at debug info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124490