[PATCH] D16727: [Profiling] Write out sparse indexed profiles

[Nathan Slingerland via llvm-commits]

slingn added a comment.

Making sparse the default would change the output of llvm-profdata show since zero count functions would no longer be listed. Is that a concern?


================
Comment at: include/llvm/ProfileData/InstrProfWriter.h:64
@@ -61,2 +63,3 @@
 private:
+  bool checkIfDataUseful(const ProfilingData &PD);
   void writeImpl(ProfOStream &OS);
----------------
I'm also not sure about 'Useful' as the name - since zeros are useful - just not necessary to write out.

How about shouldEncodeData()?

================
Comment at: lib/ProfileData/InstrProfWriter.cpp:153
@@ -152,1 +152,3 @@
 }
+void InstrProfWriter::setOutputSparseness(bool Sparse) {
+  this->Sparse = Sparse;
----------------
Since this takes a boolean I would suggest naming it setSparseOutput(bool Sparse)

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:231
@@ -230,1 +230,3 @@
 
+  cl::opt<bool> OutputSparse("sparse", cl::init(true),
+      cl::desc("Generate a sparse profile (not meaningful for -sample)"));
----------------
I would invert this to say 'only meaningful for instrumentation profiles'.


http://reviews.llvm.org/D16727