RFR: 8254108: ciReplay: Support incremental inlining

Dean Long dlong at openjdk.java.net
Tue Nov 16 22:38:34 UTC 2021


On Tue, 16 Nov 2021 15:45:15 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> This patch adds support to explicitly apply incremental inlining when replay compiling a method if the original compilation of the method was also incrementally inlined. We write a new value when dumping the inline tree to indicate if an inlinee was incrementally inlined (`= 1`) or not (`= 0`). 
> 
> To implement this, I updated the `REPLAY_VERSION` to 2 and additionally added a test to verify that old replay file versions are still working. I added some support to modify/remove version numbers of generated replay files in tests. I also refactored the test added by JDK-8275868 to reuse some of the methods.
> 
> Thanks,
> Christian

Changes requested by dlong (Reviewer).

src/hotspot/share/ci/ciReplay.cpp line 775:

> 773:               // Pending exception?
> 774:               break;
> 775:             }

I don't see how a pending exception is possible here, given the check at L763, and parse_int() doesn't throw any.
What do you think about not calling parse_int() if _version < 2, that way there is no error to ignore?

src/hotspot/share/opto/bytecodeInfo.cpp line 384:

> 382:     return false;
> 383:   }
> 384:   if (should_not_inline(callee_method, caller_method, caller_bci, NOT_PRODUCT_ARG(should_delay) profile)) {

How about a comment for these two calls saying replay may override "should_delay"?

src/hotspot/share/opto/bytecodeInfo.cpp line 609:

> 607:     InlineTree* callee_tree = build_inline_tree_for_callee(callee_method, jvms, caller_bci);
> 608:     if (should_delay || AlwaysIncrementalInline) {
> 609:       callee_tree->set_late_inline();

It took me a while to figure out why this is needed: for replay.  It bothers me a little that AlwaysIncrementalInline is check here and again in the caller.  If the replay file sets should_delay to false, then we shouldn't let AlwaysIncrementalInline to force it to true, right?  So I'm wondering if it would be better to pre-set should_delay to true in the caller if AlwaysIncrementalInline is true.

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

PR: https://git.openjdk.java.net/jdk/pull/6413


More information about the hotspot-compiler-dev mailing list