[PATCH] D142747: [Pseudo Probe] Do not instrument EH blocks.

Fri Jan 27 11:04:27 PST 2023

hoy added inline comments.

================
Comment at: llvm/lib/Transforms/IPO/SampleProfileProbe.cpp:300
   for (auto &BB : *F) {
-    BlockProbeIds[&BB] = ++LastProbeId;
+    if (!KnownColdBlocks.contains(&BB))
+      BlockProbeIds[&BB] = ++LastProbeId;
----------------
wlei wrote:
> Since this skipped some BBs and might assign different id to the same BB, it seems to me this will cause some profile matching issue? say, it's good for on-demand profiling, but for the profile in the production, it still used the old number of BB.
This is very good point! Yeah, the downwards compatibility is being screwed up there. I guess we can give blocks the same original id, but then it's always going to be like that. Fortunately this change is so far only needed for llvm-15 and there should be no legacy profiles.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142747