[PATCH] D69471: [Coverage] Revise format to reduce binary size

Vedant Kumar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 4 15:08:17 PST 2019


vsk reopened this revision.
vsk marked an inline comment as done.
vsk added a comment.
This revision is now accepted and ready to land.

I reverted this due to failures on Windows that I did not encounter in local testing. I suspect that there's an error in the coverage parsing logic, as the same binary coverage data parses successfully locally, but not on some armv7 bots.



================
Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:53-56
+    Error E =
+        zlib::compress(FilenamesStr, CompressedStr, zlib::BestSizeCompression);
+    if (E)
+      llvm_unreachable("Unexpected failure in zlib::compress");
----------------
efriedma wrote:
> vsk wrote:
> > rnk wrote:
> > > I think the shorter pattern for this is `cantFail(zlib::compress(...))`.
> > Both sound reasonable, I'll pick the shorter spelling.
> cantFail and report_bad_alloc_error have substantially different semantics. In particular, for non-Asserts builds, cantFail is a no-op, which doesn't seem appropriate for an API that fails on out-of-memory.
I see, I'll revise this to use report_bad_alloc_error in the next update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69471





More information about the cfe-commits mailing list