[PATCH] D132601: [llvm-profdata] Improve profile supplementation
Rong Xu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 26 10:31:41 PDT 2022
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
More information about the llvm-commits
mailing list