RFR: 8257211: C2: Enable call devirtualization during post-parse phase [v3]

[Nils Eliasson]

On Wed, 2 Dec 2020 08:46:14 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> As of now, call devirtualization is performed only during parsing. After parsing is over, virtual calls stay unchanged even if there's enough information available to successfully devirtualize them.
>> 
>> Proposed patch enhances incremental inlining with virtual call support. The implementation is similar to how late inlining through MethodHandle linkers works: `CallGenerator` is linked from related `CallJavaNode` and once the call becomes eligible for devirtualization, it is put on the late inline list for future processing. It enables inlining through the call, but also even when inlining doesn't happen, devirtualization is beneficial anyway: it strength-reduces a virtual call to a static call (optimised virtual). 
>> 
>> Also, the patch introduces a dedicated pass over inline list very late in the pipeline specifically to strength reduce virtual calls to static ones w/o even attempting inlining. And it covers late inlining of MethodHandle linkers along the way.
>> 
>> Initial version of the patch has been integrated into `panama-dev` (`vectorIntrinsics` branch) in July, 2020 [1] and since then it went through multiple rounds of testing and a number of bugs were fixed.
>> 
>> (Original motivation for the work was Vector API which suffered severe performance regression after major API and implementation refactoring (`vector-unstable` branch), but it turned out it benefits ordinary Java code as well.) 
>> 
>> Testing:
>> - [x] hs-tier1-6 w/ and w/o `-XX:+AlwaysIncrementalInline` and `-XX:+StressMethodHandleLinkerInlining`
>> 
>> Thanks!
>> 
>> [1] https://hg.openjdk.java.net/panama/dev/rev/5b601a43ac88
>
> Vladimir Ivanov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Minor cleanup in comments

Great work!

Only minor comments.

Reviewed.

src/hotspot/share/opto/phaseX.cpp line 1121:

> 1119: #ifdef ASSERT
> 1120:   if (modified_list != NULL) {
> 1121:     while (modified_list->size()) {

Nitpicking here: 
Change to "while (modified_list->size() != 0) {"

src/hotspot/share/opto/multnode.cpp line 66:

> 64: }
> 65: 
> 66: ProjNode* MultiNode::proj_out_or_null(uint which_proj, bool is_io_use) const {

This MultiNode::proj_out_or_null and the MultiNode::proj_out_or_null above are very similar but with a few differences. The only use of this method has is_io_use false.

I feel that there might be a risk of mixup in the future. Perhaps a different name can help? I don't have a better solution so I don't require you to fix this.

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

Marked as reviewed by neliasso (Reviewer).

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