[PATCH] D22120: Move inline threshold related flags to InlineSimple.cpp

[Xinliang David Li via llvm-commits]

On Fri, Jul 22, 2016 at 2:37 PM, Easwaran Raman <eraman at google.com> wrote:

> eraman added inline comments.
>
> ================
> Comment at: lib/Transforms/IPO/InlineSimple.cpp:138
> @@ +137,3 @@
> +  // Explicitly specified -inline-threshold overrides Threshold.
> +  return new SimpleInliner(DefaultInlineThreshold.getNumOccurrences() > 0
> +                               ? DefaultInlineThreshold
> ----------------
> davidxl wrote:
> > eraman wrote:
> > > davidxl wrote:
> > > > Why? If Threshold is explicitly passed in, should it be honored with
> highest precedence?
> > > This is the current behavior : see the code in updateThreshold. It
> also makes sense. If the tool user sees a flag and explicitly passes a
> value to it, they expect the value to take effect. Even if you disagree
> with this, that should be a separate change.
> > OK -- if this is intended as NFC. On the other hand how do you plan to
> handle fine grain control of different instances of the inliner ? (LTO,
> thinLTO, IRPGO)?
> If we want their thresholds to be controlled by a flag, we need a separate
> flags and create separate entry points. For example, we should add a
> createPrePnliner pass that should set the threshold using a
> -preinline-threshold flag.
>

It appears to me that if the creator API is used in a way that 'explicit'
threshold is passed in, it is already selected via some other options which
should take precedence over the generic tuning option -inline-threshold.
Otherwise, the default interface or the one taking opt level should be
used.  Based on this observation, I think the right behavior is not to
override the passed in threshold.

David


>
>
> https://reviews.llvm.org/D22120
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160722/d32d7cc3/attachment.html>