RFR: 8319850: PrintInlining should print which methods are late inlines
Dean Long
dlong at openjdk.org
Fri Nov 8 03:12:46 UTC 2024
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.
I agree with Roland, rather that overwriting the old information, it would be nice to append to it. Unfortunately this late inlining print support is a bit complicated and also a bit broken, I discovered recently. It could probably use a cleanup. I hit one assert because there was no message printed in do_late_inline_check when allow_inline was set to true. When I investigated that, I discovered that print_inlining_commit() will happily append a new message next to an old message on the same line. Something is going wrong with the logic in print_inlining_update() when cg() is null. I'm wondering if we could simplify things by placing the stringStream inside the CallGenerator.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/21899#issuecomment-2463671322
More information about the hotspot-compiler-dev
mailing list