[PATCH] D49412: Enrich inline messages

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: include/llvm/Transforms/Utils/Cloning.h:218
+/// describes a reason why it is decided to not inline.
+struct InlineResult {
+  const char* message = nullptr;
----------------
yrouban wrote:
> tejohnson wrote:
> > I think this would make more sense in InlineCost.h
> InlineCost.h relates to //Analysis// but InlineResult is a result of a //transformations//.
> In other words all users of InlineResult reside in lib/Transforms.
> But I do not insist.
It seems to mostly be related to analysis though, so Cloning.h seems to be the wrong place IMO. Mostly this structure is created by InlineCost.cpp (although I see it is also created by InlineFunction.cpp in some cases).

Also, I believe it is wrong to include a Transforms header into an Analysis cpp file, whereas the reverse is fine (I just checked and this would be the first Transforms header in any Analysis cpp file).


================
Comment at: lib/Transforms/IPO/Inliner.cpp:442
              << NV("Callee", Callee) << " not inlined into "
-             << NV("Caller", Caller) << " because too costly to inline (cost="
-             << NV("Cost", IC.getCost())
-             << ", threshold=" << NV("Threshold", IC.getThreshold()) << ")";
+             << NV("Caller", Caller) << " because too costly to inline ("
+             << IC << ")";
----------------
yrouban wrote:
> tejohnson wrote:
> > nit: suggest changing to:
> >              << NV("Caller", Caller) << " because too costly to inline with " << IC;
> > so that you don't end up with 2 levels of nested parentheses in messages.
> do you mind if I put ':' before inline cost in both cases?
>   << " because it should never be inlined: " << IC;
>   << " because too costly to inline: " << IC;
> 
Colon looks good here in the too costly to inline case. Slight preference for the parentheses in the earlier case, just around the "cost=never", to avoid the 2 sets of ":". I.e. so that it looks like:
foz not inlined into bar because it should never be inlined (cost=never): noinline function attribute  (hotness: 30)


https://reviews.llvm.org/D49412