[PATCH] D36726: [Inliner] Teach the inliner to propagate attributes that have specific effects on inlining thresholds when we happen to inline into the entry (extended) basic block.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc created this revision.
Herald added a subscriber: mcrosier.

The fundamental idea here is to address a fragility in the bottom-up
inliner. When inlining a call edge it can *erase* information which
would otherwise very dramatically impact the inlining heuristics. We've
seen this before with noalias and alignment parameter attributes and
addressed them with scoped aliasing information and assumptions
respectively.

However, another aspect of information that may be lost are the
`inlinehint` and `cold` attributes which can have a significant impact
on inlining thresholds. Specifically, there is an assymetry in how these
end up being handled depending on whether we happen to successfully
defer inlining during the bottom-up walk or not.

If we defer inlining during the bottom-up walk, we will *also* end up
"propagating" the `inlinehint` and `cold` up one level of the call graph
by inlining the wrapper function first and then re-considering the new
call edge which still has these attributes.

If we *don't* defer inlining, then we will inline into the function and
lose these attributes. Later on, we may simplify away everything else
and end up forming an *exact copy* of the original function body. But
because the inlining step lost these attributes this copy of the
original function body will inline substantially differently (either too
little in the case of `inlinehint` or too much in the case of `cold`).

With this patch, we try to detect the obviously safe cases where we are
inlining into a callsite that could easily simplify into a trivial
wrapper by looking for callsites in the (extended) entry block. This
also makes it easy to ensure that the callsite is reached for the case
of the `cold` attribute that may be a dynamic property.

The goal here is mostly to make inlining decisions a bit more
predictable. We should still work on improving the logic around
deferring inlining during the bottom-up walk to find better inlining
candidates, this should just be a useful backstop to ensure that in
addition to missing the deferral opportunity we don't *also* erase
valuable inlining attributes.

An example of a case today that is highly surprising (and motivates this
patch) would be a trivial lambda function passed to STL algorithms. The
operator()s on this lambda object may not be marked with `inlinehint`,
but it may be a trivial wrapper around some other function which is
marked with `inlinehint`. It is then surprising when the inliner
"erases" this information and makes different (in either direction)
inlining decisions through the wrapper than it would through a direct
call to the member function. This pattern has actually shown up in a few
benchmarks.

So far, initial benchmarking has shown no substantial performance swings
either direction, but I'm still collecting more precise numbers. I just
wanted to get this out for review.

Depends on https://reviews.llvm.org/D36722.


https://reviews.llvm.org/D36726

Files:
  lib/Transforms/IPO/Inliner.cpp
  test/Transforms/Inline/inline-attr-prop.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36726.111106.patch
Type: text/x-patch
Size: 10700 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170815/9b05d099/attachment.bin>