[XXS] C1 misses to dump a reason when it inlines successfully

[Liu Xin]

Hi, Tobias & Martin,

The original message even didn't say inline succeed or not. When I see
those, I have to dig into the source code. I spent even more time because I
didn't realize that those messages are from C1. That's my standpoint and
motivation as a new hotspot developer.
What C2 inliner emits "inline (hot)"  is not very informative neither, but
at least developers know that this callee is inlined. I gave up "by the
rules of C1". I think "inline" is fine. developers can also tell difference
from C2's output.

I agree with you about the default parameter.  Failed attempts should
always have a reason. Developers can inspect them and make adjustment.

Here is a new revision. Could you take a look?
https://cr.openjdk.java.net/~xliu/8234541/01/webrev/

sample output is like this.
                              @ 1   java.lang.String::isLatin1 (19 bytes)
inline
                              @ 12   java.lang.StringLatin1::charAt (28
bytes)   inline
                                @ 15
 java/lang/StringIndexOutOfBoundsException::<init> (not loaded)   not
inlineable
                              @ 21  java/lang/StringUTF16::charAt (not
loaded)   not inlineable
                              @ 15
 java/lang/StringIndexOutOfBoundsException::<init> (not loaded)   not
inlineable
                              @ 3   java.util.stream.ReduceOps::makeInt (18
bytes)   inline
                                @ 1   java.util.Objects::requireNonNull (14
bytes)   inline
                                  @ 8
java.lang.NullPointerException::<init> (5 bytes)   don't inline Throwable
constructors
                                @ 14   java.util.stream.ReduceOps$6::<init>
(16 bytes)   inline
                                  @ 12
java.util.stream.ReduceOps$ReduceOp::<init> (10 bytes)   inline
                                    @ 1   java.lang.Object::<init> (1
bytes)   inline
                              @ 6
java.util.stream.AbstractPipeline::evaluate (94 bytes)   callee is too large
thanks,
--lx



On Thu, Nov 28, 2019 at 6:53 AM Doerr, Martin <martin.doerr at sap.com> wrote:

> Hi,
>
> I agree with Tobias. " by the rules of C1" doesn't add any useful
> information IMHO.
> But I'd be fine with just adding "inline" to avoid the confusion.
>
> > Anyway, with your change, all callers now pass a msg argument and
> > therefore the null handling logic
> > should be removed (replaced by an assert). Also, the default value for
> the
> > argument can be removed.
> +1
>
> Best regards,
> Martin
>
>
> > -----Original Message-----
> > From: hotspot-compiler-dev <hotspot-compiler-dev-
> > bounces at openjdk.java.net> On Behalf Of Tobias Hartmann
> > Sent: Donnerstag, 28. November 2019 15:04
> > To: Liu Xin <navy.xliu at gmail.com>; hotspot-compiler-dev at openjdk.java.net
> > Subject: Re: [XXS] C1 misses to dump a reason when it inlines
> successfully
> >
> > Hi,
> >
> > I'm still not convinced that this message adds any useful information
> but let's
> > see what other
> > reviewers think.
> >
> > Anyway, with your change, all callers now pass a msg argument and
> > therefore the null handling logic
> > should be removed (replaced by an assert). Also, the default value for
> the
> > argument can be removed.
> >
> > Best regards,
> > Tobias
> >
> > On 22.11.19 19:09, Liu Xin wrote:
> > > hi, Reviewers,
> > >
> > > Could you review this extremely small change?
> > > Bugs: https://bugs.openjdk.java.net/browse/JDK-8234541
> > > Webrev: https://cr.openjdk.java.net/~xliu/8234541/00/webrev/
> > >
> > > When I analyzed PrintInlining, I was confused by the inline message
> > without
> > > any detail. It's not easy for developer to tell if this method is
> inlined
> > > or not. This patch add a comment "inline by the rules of C1".
> > >
> > > I would like to add an explicit reason, but there's no decisive reason
> in
> > > GraphBuilder::try_inline_full. It just passes all restrict rules. Any
> other
> > > suggestion would be appreciated.
> > >
> > > Thanks,
> > > --lx
> > >
>