zequanwu marked an inline comment as done.
zequanwu added inline comments.
================
Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:152
+static MutableArrayRef<CounterMappingRegion>
+mergeSkippedRegions(MutableArrayRef<CounterMappingRegion> MappingRegions) {
+ SmallVector<CounterMappingRegion, 32> MergedRegions;
----------------
vsk wrote:
> zequanwu wrote:
> > vsk wrote:
> > > This seems like a lot of complexity to add to handle a narrow case. Is it
> > > necessary to merge skipped regions early in the process? Note that llvm's
> > > SegmentBuilder takes care of merging regions at the very end.
> > Merging at here is to avoid duplicate output for option
> > `-dump-coverage-mapping`. Like the following example to avoid `Skipped,File
> > 0, 2:3 -> 4:9 = 0` and `Skipped,File 0, 2:15 -> 2:25 = 0` to be outputted
> > at the same time. They should be merged into 1 region `Skipped,File 0, 2:3
> > -> 4:9 = 0`
> > ```
> > File 0, 1:12 -> 6:2 = #0
> > Skipped,File 0, 2:3 -> 4:9 = 0
> > Skipped,File 0, 2:15 -> 2:25 = 0
> > 1| 1|int main() {
> > 2| | #ifdef TEST // comment
> > 3| | int x = 1;
> > 4| | #endif
> > 5| 1| return 0;
> > 6| 1|}
> > ```
> > So, we need to do it early.
> I see that there are duplicate 'Skipped' regions emitted in this case, but
> I'm not sure what problems this causes. For testing, we could check that both
> skipped regions are emitted, or perhaps only check that the outermost skipped
> region is emitted and ignore others. Is there some other kind of hard error
> (like an assert) caused by having nested skipped regions? If not, perhaps
> it's not worth it to merge them.
No, it doesn't cause any error.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83592/new/
https://reviews.llvm.org/D83592
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
