[PATCH] D98213: [InlineCost] Enable the cost benefit analysis on FDO

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

> Note the size here is not static code size (it excludes cold code). It is actually a proxy to model the runtime cost due to increased instruction footprint (icache pressure). The multiplier's role is to make the savings and 'size' cost comparable in terms of unit. The cycle name here is also counted at IR level, so not at low level.

I understand that, but it still doesn't make the comparison of different units any better. Introducing a scaling factor is irrelevant, it's just an arbitrary scalar.

> Note that FDO is intended to be improve performance and the compile time budget is larger. This all seems WAI to me. If size is important, use -Os or -Oz, or even better use MLGO.

The cases we've seen involve orders of magnitude increase, e.g. 100x bigger size and compile time. Not working as intended.

  // We use 128-bit APInt here to avoid potential overflow.  This variable
  // should stay well below 10^^24 (or 2^^80) in practice.  This "worst" case
  // assumes that we can avoid or fold a billion instructions, each with a
  // profile count of 10^^15 -- roughly the number of cycles for a 24-hour
  // period on a 4GHz machine.

If you potentially need a 128 bit integer to store your "cycle savings", you should not be using that value to compare against a size cost. A sufficiently good "saving" will absolutely override a huge size cost.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98213