[PATCH] D81981: [PGO] Supplement PGO profile with Sample profile

[Wei Mi via Phabricator via llvm-commits]

wmi marked 5 inline comments as done.
wmi added inline comments.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:293
 
+static void writeInstrProfile(StringRef OutputFilename,
+                              ProfileFormat OutputFormat,
----------------
davidxl wrote:
> this refactoring can also be committed independently
Done in https://reviews.llvm.org/D83521


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:553
+  std::unique_ptr<WriterContext> WC;
+  // Make sure Inputs[i] is sample profile and Inputs[i - 1] is
+  // instrumentation profile.
----------------
davidxl wrote:
> make sample file path as the part of the option, so there is no need to handle the ordering.
Indeed that will save the ordering handle logic, but I want to use weighted_input to scale the count in sample profile to be roughtly the same as the count in instr profile. To support -supplement-instr-with-sample=<weight>,<filename> will be a little weird and increase complexity. 


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:568
+  adjustInstrProfiles(WC, Reader,
+                      Inputs[i].Weight / (double)Inputs[1 - i].Weight,
+                      EarlyInlineSizeThreshold, BaseScaleFunction);
----------------
davidxl wrote:
> Are these two weights comparable? 
Yes, given "-weighted-input=2, instr_profile -weighted-input=3, sample_profile", that means we want to scale the count in sample profile by 3/2 before update the entry in instr profile.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:872
+  cl::opt<std::string> BaseScaleFunction(
+      "base-scale-function", cl::init(""), cl::Hidden,
+      cl::desc("When supplementing an instrumentation profile with sample "
----------------
davidxl wrote:
> Is this flag tested?
Good point, add tests for this flag and the flag early-inline-size-threshold


Repository:
  rL LLVM

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

https://reviews.llvm.org/D81981