[PATCH] D150466: profilie inference changes for stale profile matching

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SampleProfileInference.cpp:1230
+  }
+  assert(NumExitBlocks > 0 && "cannot find exit blocks");
+
----------------
how about noreturn function, or functions with an infinite loop? These are not important functions to optimize, but at least we shouldn't fail on those cases? 


================
Comment at: llvm/lib/Transforms/Utils/SampleProfileInference.cpp:1339
+  // Quit early for functions with a single block or ones w/o samples
+  if (Func.Blocks.size() <= 1 || !HasSamples)
+    return;
----------------
spupyrev wrote:
> hoy wrote:
> > Wondering in what cases a function doesn't come in with samples? Thought we should have bailed out on a higher level in the sample loader.
> This is going to be called from BOLT in addition to sample loader. One scenario when a function doesn't have samples is when its profile data is so stale that we couldn't match a single block/jump/call.
> (I agree that check could be verified earlier but i don't see a big advantage.)
`SampleProfileInference<BT>::apply` has the same logic to check function size and whether there's sample. Better unify and avoid duplication. I also agree that it's better to check at caller side. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150466