[llvm] [llvm-profdata] Reject merging single-byte-coverage with count profiles (PR #213177)

[via llvm-commits]

gulfemsavrun wrote:

> I don't think we should add a flag to sometimes allow some type of merging to be legal. If merging two profiles is problematic, we should report an error, otherwise we should merge them if we can.
> 
> The issue is that single byte coverage is a global flag. There is no way to specify that function A uses single-byte coverage and function B uses 8-byte counters. I believe the issue we encountered is that we were merging two raw profiles where one used single-byte coverage and the other used 8-byte counters (this usually would never happen, but we mistakenly merged two raw profiles from separate builds). The fundamental issue for raw profiles is we don't know the sizes of each function's raw profile data, so we can't merge them.
> 
> @gulfemsavrun @petrhosek I assume you are merging indexed profiles (`.profdata` files) with mixed coverage and counters? It's been a while since I've looked at the code, but if both single-byte coverage and 8-byte counters are encoded as 8-bytes in the indexed profile, I think this should be fine. But when we merge these profiles, we should "upgrade" the to the 8-byte counter profile and make sure we don't emit the single-byte variant.
> 
> As for the loss in precision during the merge, by using single-byte coverage, you should already accept that profile data won't be accurate. I think we should consider a "covered" block as having a counter value of 1, or maybe some value N passed as a flag.
> 
> tl;dr; If you need this, we should relax the error for merging indexed profiles. But we should not allow merging a single-byte coverage raw profile with anything other than another single-byte coverage profile.

Your assumption about our workflow is correct: we first merge the single-byte raw profiles (`.profraw`) from Clang to produce an indexed profile (`.profdata`), merge the 8-byte raw profiles from Rust to produce a separate indexed profile, and then merge those two indexed profiles together for coverage reporting.

I agree with your suggestion:

1) Raw profiles: We can keep rejecting merges between raw profiles with different counter modes as an error.
2) Indexed profiles: We can relax the check when merging indexed profiles (`.profdata`).

https://github.com/llvm/llvm-project/pull/213177