[PATCH] D73994: [InlineCost] Relax bonus restrictions on uninlinable functions

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv created this revision.
george.burgess.iv added reviewers: davidxl, mtrofin, eraman.
Herald added subscribers: llvm-commits, dexonsmith, haicheng, hiraditya, mehdi_amini.
Herald added a project: LLVM.

We currently ignore inlining bonuses sometimes (importantly to me, `LastCallToStaticBonus`) in order to make the caller of a function more inlinable. This appears suboptimal if the caller can't be inlined in the first place.

This is an attempt to fix that suboptimality (which showed up for me most prominently with the new PM), though it's not quite finished. Before I plumb everything through, I'd like to know if anyone's strongly against a change like this.

Importantly, I can see the following downsides:

- This patch makes our inline cost depend on state of the caller, which is generally discouraged, since we try to cache these things.
- This patch basically causes the inliner to inline differently depending on whether we're LTO'ing or not, which AFAICT is new.

The last bullet is the case because, in a traditional build, I don't think we care about how inlineable `exportedFunction` is below, assuming it has no users in its TU:

  static void helper() {
    // large body
  }
  
  void exportedFunction(bool i) {
    if (UNLIKELY(i))
      helper();
  }

OTOH, with ThinLTO or LTO enabled, we might discover uses of `exportedFunction` during our link, so the inlinability of it starts to matter.

Thoughts appreciated. :)


https://reviews.llvm.org/D73994

Files:
  llvm/lib/Analysis/InlineCost.cpp
  llvm/test/Transforms/Inline/bpi-cold-inlining.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73994.242420.patch
Type: text/x-patch
Size: 5893 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200204/92858826/attachment.bin>