[PATCH] D15853: [PGO]: Simplify coverage data lowering code

Vedant Kumar via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 7 09:52:05 PST 2016


vsk added a comment.

Fwiw the previous version of this patch looked good to me.

In this version 'getCoverageNamesVarName()' has a new meaning. It should probably be renamed (maybe 'getUnusedPGONamesVarName()'), and its comment should be updated.

I see that `lowerCoverageData()` sets the section information and alignment for names. With this change, it no longer handles records for which `addFunctionMappingRecord(isUsed = true)`. It looks like these names will no longer be materialized into __llvm_prf_names. This doesn't seem right to me. Could you explain why it works? This code is a bit new to me...


================
Comment at: lib/CodeGen/CoverageMappingGen.h:75
@@ -74,1 +74,3 @@
+                                const std::string &CoverageMapping,
+                                bool isUsed = true);
 
----------------
Nit: Parameter names should be capitalized.


http://reviews.llvm.org/D15853





More information about the cfe-commits mailing list