[PATCH] D28965: [PGO] Value profile for size of memory intrinsic calls

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

llvm-profdata needs test cases for profile reading and writing

1. show command
2. merge command in binary and in text format



================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:79
     cl::init(1.0));
+cl::opt<unsigned> MemOPSizeSmallVal(
+    "memop-size-small",
----------------
Perhaps make it a string option with both start and end values specified:

MemOpSizeValueRange 


================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:85
+    "memop-size-large",
+    cl::desc("Set the threshold value to be profiled seperately"),
+    cl::init(8192));
----------------
The description is not accurate.


================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:482
       InstrProfSymtab &Symtab = Reader->getSymtab();
       InstrProfWriter::writeRecordInText(Func, Symtab, OS);
       continue;
----------------
Add a test case to show text format dump works expected.


================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:542
       }
+      if (ShowMemOPSizes && MemOPSize > 0) {
+        uint32_t NS = Func.getNumValueSites(IPVK_MemOPSize);
----------------
Collect summary statistics and dump it at the end -- similar to IC statistics.


================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:631
       cl::desc("Show indirect call site target values for shown functions"));
+  cl::opt<bool> ShowMemOPSizes(
+      "memop-sizes", cl::init(false),
----------------
This needs to be documented as other user facing options.


https://reviews.llvm.org/D28965