[PATCH] D136627: [SampleFDO] Compute profile mismatch metrics

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2106
+    const LineLocation &Loc = I.first;
+    if (Loc.LineOffset & InvalidLineOffsetBit)
+      continue;
----------------
hoy wrote:
> Should invalid line offset also be considered mismatched samples? The samples will discarded any way. 
> 
> BTW, sort of having an impression that the invalid line offset checking is used anywhere else. Can we unify the usage if that's the case?
>From my observation, the invalid(negative) offset will never be matched to any offset in the IR, so we actually need to remove it from the profile rather than report as mismatching. Recalling we tried to remove this offset in llvm-profgen, however, it caused a regression(likely because it affect the sample hot cutoff), so we chose to keep it. As for the mismatch,  I'm thinking to report the samples that's caused by the real stale profile issue, this might mislead the user. 

> BTW, sort of having an impression that the invalid line offset checking is used anywhere else. Can we unify the usage if that's the case?
yeah, that's from llvm-profgen, but that diff is reverted.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2203
+  if (ReportProfileStaleness)
+    MatchingManager->matchProfiles();
+
----------------
hoy wrote:
> nit: rename `matchProfiles` something like `detectProfileMismatch` directly? The name of the switch sounds like only mismatch detection should be done here.
Fixed, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136627