[PATCH] D124490: [InstrProf] Minimal Block Coverage

Thu May 19 18:54:58 PDT 2022

gulfem added a comment.

In D124490#3522697 <https://reviews.llvm.org/D124490#3522697>, @ellis wrote:

> 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.

That's great!

> 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.

`CoverageMappingGen.cpp` adds counters while traversing AST. For ex, this is how counters are added for an if statement.
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CoverageMappingGen.cpp#L1383
I'm currently building the prototype for using boolean counters in coverage, and I'm planning to upload a WIP review soon.
After that, we can discuss further whether `BlockCoverageInference` can be generalized.

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

How are you planning to use `block coverage` for `PGO`?
In some of the tools that we show source-based coverage, we show whether a line is covered by the executed tests and do not show how many times each line is executed. For this specific purpose, we want to use single byte counters.
Although I'm not that familiar with PGO implementation, it seems like number counters might be more beneficial for making optimization decisions for PGO.  Are there specific optimizations that you are planning to use `block coverage`?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124490