[PATCH] D132601: [llvm-profdata] Improve profile supplementation

[Rong Xu via Phabricator via llvm-commits]

xur added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/InstrProf.h:844
+  }
+  void setPseudoCount(bool IsHot) {
+    uint64_t Val = (uint64_t)(IsHot ? HotFunctionVal : WarmFunctionVal);
----------------
davidxl wrote:
> pass CountPseudoKind
Ack.


================
Comment at: llvm/lib/ProfileData/InstrProf.cpp:721
+  CountPseudoKind ThisKind = getCountPseudoKind();
+  // This is less optimial as one of the proifles might not be
+  // a peseudo one and might have better information. But we don't
----------------
davidxl wrote:
> typo: proifiles
Ack.


================
Comment at: llvm/lib/ProfileData/InstrProf.cpp:722
+  // This is less optimial as one of the proifles might not be
+  // a peseudo one and might have better information. But we don't
+  // have summary information here. We cannot tell if the non-pseudo profile
----------------
davidxl wrote:
> typo peseudo
Ack.


================
Comment at: llvm/lib/ProfileData/InstrProf.cpp:725
+  // is better. We just use pesudo profile here.
+  if (OtherKind != NotPseudo || ThisKind != NotPseudo) {
+    bool IsHot = (OtherKind == PseudoHot || ThisKind == PseudoHot);
----------------
davidxl wrote:
> We should probably assume/require both profiles are (or not) supplemented, and always take the non pseudo one.
> 
> Or do not support merge with supplemented profile (i.e. only supplement after merging).
Supplementing can produce a normal profile (only scaling), or a profile with pseudo counts.
So it's hard to force not merging supplemented profiles.

Probably the fist suggestion is the way to go: give a hard error when merging profile with pseudo counts and without pseudo counts. 


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

https://reviews.llvm.org/D132601