[GCOV] Don't add a useless block in the entry
AcceptedPublic

Authored by calixte on Fri, Sep 14, 5:53 AM.

Details

Reviewers
vsk
marco-c
Summary

Currently, we split the entry block of a function to be sure to have at least one edge. In fact it's useless since we can add this edge in gcno/gcda without creating it.
There are several pros to do that:

  • slightly reduce gcno/gcda size (close to nothing...).
  • fix a crash when there are several CU attached to a module (the entry block was splitted several times)
  • when using -femit-coverage-data the entry was not splitted since it's currently done when emitting coverage notes, so no instrumentation added when there was only on block.

Diff Detail

calixte created this revision.Fri, Sep 14, 5:53 AM

I'm not really familiar with the gcov file format, but this change looks reasonable.

lib/Transforms/Instrumentation/GCOVProfiling.cpp
670

succ_empty()

There are four possible terminators for a block with no successors: ret, unreachable, resume, and cleanupret. Writing good testcases for exception handling is probably hard, but it would be nice to at least ensure we generate something sane for a function that explicitly calls exit().

test/Transforms/GCOVProfiling/multiple-cus.ll
5

(Needs to be fixed.)

marco-c accepted this revision.Thu, Sep 20, 2:42 AM

This looks good to me, modulo the changes requested by efriedma (and the additional tests to write).

It might be useful to stress test the patch on our CI to see if there are corner cases we haven't thought of.

This revision is now accepted and ready to land.Thu, Sep 20, 2:42 AM
marco-c added inline comments.Thu, Sep 20, 2:42 AM
lib/Transforms/Instrumentation/GCOVProfiling.cpp
558

Nit: I'd keep this the other way around, to have a smaller diff