[PATCH] D101780: [CoverageMapping] Handle gaps in counter IDs for source-based coverage

[Rich Kadel via Phabricator via llvm-commits]

richkadel added a comment.

> While it isn't a hard requirement that the source -> coverage mapping become immutable before optimization begins, that is the traditional approach taken by clang & swiftc. (We nod towards this in https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#impact-of-llvm-optimizations-on-coverage-reports,

@vsk - Just so I'm clear, are you saying that every Counter and Expression generated is guaranteed to be added to the coverage map (so there are no index gaps), even if the counter is part of an unreachable (and therefore eliminated) branch/block?

And, if so, I assume it's OK for a dead branch/block to be eliminated (so the `instrprof.increment` call for that branch's counter is never added.) But even if the code to increment that counter is not in the code, the coverage map includes the counter. Right?

I recently added support for reporting code regions for dead blocks, but I am replacing them with Zero counters. If I'm understanding you correctly, then I may be able to just retain the original counter id and/or expression and add them to the map instead of converting them to Zero counters, and therefore meet your expectations.

> I see two options for moving forward:
>
> 1. Have llvm be more permissive about the number of counters when loading sparse .profdata (this patch)
> 2. Have llvm be less permissive (assert that #counters < #regions), document the new requirement and change the Rust FE to emit counters before DCE
>
> I think (2) would lead to less complexity overall in both Rust & llvm, but both sound reasonable to me. Wdyt?

Unless I misunderstand the above, then yes, option 2. And the assertion error message needs to explain the problem (something like, "non-contiguous counter IDs are not allowed")  if you discover a coverage map has counter ID gaps or expressions that refer to missing counters.

Thanks for pointing this out and let me know what I have wrong here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101780