RFR: 8319850: PrintInlining should print which methods are late inlines

[Roland Westrelin]

On Tue, 5 Nov 2024 10:19:51 GMT, theoweidmannoracle <duke at openjdk.org> wrote:

> In https://github.com/openjdk/jdk/pull/16595 @caojoshua previously suggested changes to indicate which calls were inlined late, when printing inlines. This PR re-introduces the changes from the previously closed PR and fixes a minor issue where asserts were triggered.
> 
> Concerns were raised by @rwestrel in the previous PR:
> 
>> When InlineTree::ok_to_inline() is called, some diagnostic message is recorded for the call site. Do I understand right that with this patch, if the call is inlined late, then that message is dropped and replaced by a new "late inline.." message? If that's the case, isn't it the case that sometimes the InlineTree::ok_to_inline() has some useful information that's lost when late inlining happens?
> 
> As already pointed out in the PR by @caojoshua, this does not matter for string/methodhandle/vector/boxing late inlines, as they are [only performed if ok_to_inline() returns true](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/doCall.cpp#L189). This is also the only call to ok_to_inline().  
> 
> The only other location, where late inline call generators are created, are calls to CallGenerator::for_late_inline_virtual(), which creates a LateInlineVirtualCallGenerator. LateInlineVirtualCallGenerator (introduced in https://github.com/openjdk/jdk/pull/1550) does not really perform inlining but rather performs strength reduction from virtual to static calls. As can be verified by running the according test `test/hotspot/jtreg/compiler/c2/irTests/TestPostParseCallDevirtualization.java`, this does not affect the printing for inlining:
> 
> 
> 5022 1026       3       compiler.c2.irTests.TestPostParseCallDevirtualization::callHelper (7 bytes)   made not entrant
>                               @ 1   compiler.c2.irTests.TestPostParseCallDevirtualization$I::method (0 bytes)   failed to inline: virtual call
> 
> 
> Thus, as far as I can tell, the proposed changes by @caojoshua do not lose any useful information about why no inlining prior to the late inlining occurred.

With a simple test case:

 public static void main(String[] args) {
        for (int i = 0; i < 20_000; i++) {
            test1();
        }
    }

    private static void test1() {
        inlined1();
    }

    private static void inlined1() {
    }
}


Without your patch:


$ java -XX:-TieredCompilation -XX:-UseOnStackReplacement -XX:-BackgroundCompilation -XX:+PrintCompilation -XX:CompileOnly=TestLateInlining::test1 -XX:CompileCommand=quiet -XX:+PrintInlining -XX:+AlwaysIncrementalInline TestLateInlining
87    1     n       jdk.internal.vm.Continuation::enterSpecial (native)   (static)
87    2     n       jdk.internal.vm.Continuation::doYield (native)   (static)
92    3    b        TestLateInlining::test1 (4 bytes)
                            @ 0   TestLateInlining::inlined1 (1 bytes)   inline (hot)


With your patch:


$ java -XX:-TieredCompilation -XX:-UseOnStackReplacement -XX:-BackgroundCompilation -XX:+PrintCompilation -XX:CompileOnly=TestLateInlining::test1 -XX:CompileCommand=quiet -XX:+PrintInlining -XX:+AlwaysIncrementalInline TestLateInlining
86    1     n       jdk.internal.vm.Continuation::enterSpecial (native)   (static)
86    2     n       jdk.internal.vm.Continuation::doYield (native)   (static)
92    3    b        TestLateInlining::test1 (4 bytes)
                            @ 0   TestLateInlining::inlined1 (1 bytes)   late inline


I think it would be nice to preserve the "inline (hot)" part of the first input as it's the reason for inlining. There can be other reason for inlining (not many from a quick look at the code) but, who knows, there could be more in the future. 

Also, having a test case would be useful.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21899#issuecomment-2459009598