[PATCH] D145516: [Inliner] Avoid excessive inlining through devirtualised calls

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

Interesting -- I've dug into the cost modelling, and I think you're correct that there's an issue, but it's even less enjoyable. It looks like the effect of the branch-weights successfully suppresses `longname` being inlined into `rec_call_to_longname` because it's a cold callsite, in the first instance. However, once `rec_call_to_longname` is inlined into `a`, the same computations round all the frequencies to zero, and inlining starts to happen repeatedly through both `rec_call_to_longname` and `longname`, giving the exponential behaviour. Without the branch weights, inlining `longname` into `rec_call_to_longname` creates a directly recursive function that never gets inlined again.

My understanding of inlining is that if we've rejecting inlining at a call site because of the cost, it would be very unexpected if inlining the call-site lowered the cost. Thus the cost-modelling changing it's mind is the most surprising behaviour and is most likely to be wrong. It's another area I'm unfamiliar with that I'll try digging into, the relevant function is `InlineCostCallAnalyzer::isColdCallSite` [0]. I'll probably be able to work it out, but here's a dump of the relevant state just in case someone else knows a graceful solution -- in the first instance on the linked line,

- ColdProb is a fixed-point representation of 0.02f,
- CallSiteFreq is integer value 8,
- CallerEntryFreq is integer value 450,
- Return-value of `CallerEntryFreq * ColdProb` is 9

I assume others can interpret the following branch probability summary:

  block-frequency: rec_call_to_longname
  =====================================
  reverse-post-order-traversal
   - 0: 
   - 1: 
   - 2: 
  loop-detection
  compute-mass-in-function
   - node: 
    => [ local  ] weight = 38171822, succ = 
    => [ local  ] weight = 2109311826, succ = 
    => mass:  ffffffffffffffff
    => assign 048ce95bffffffff (fb7316a400000000) to 
    => assign fb7316a400000000 (0000000000000000) to 
   - node: 
    => [ local  ] weight = 2147483648, succ = 
    => mass:  048ce95bffffffff
    => assign 048ce95bffffffff (0000000000000000) to 
   - node: 
    => mass:  ffffffffffffffff
  float-to-int: min = 0.01777513977, max = 1.0, factor = 450.0667844
   - : float = 1.0, scaled = 450.0667844, int = 450
   - : float = 0.01777513977, scaled = 8.0, int = 8
   - : float = 1.0, scaled = 450.0667844, int = 450
  block-frequency-info: rec_call_to_longname
   - : float = 1.0, int = 450
   - : float = 0.017775, int = 8
   - : float = 1.0, int = 450

Wheras in the second instance, the function looks like this:

  [0] https://github.com/llvm/llvm-project/blob/96118f1b0ab7a18999a4f2199ba1ecd546c68cb8/llvm/lib/Analysis/InlineCost.cpp#L1795


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145516