RFR: 8254317: C2: Resource consumption of ConvI2LNode::Ideal() grows exponentially [v2]

[eric.1iu]

On Wed, 21 Oct 2020 17:56:04 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Roberto Castañeda Lozano has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>> 
>>  - Merge master
>>  - Generalize the fix to handle any input where AddIs are used multiple times by
>>    other AddIs, which could also lead to an exponential number of calls to
>>    ConvI2LNode::Ideal(). This is achieved by (1) reusing existing ConvI2Ls if
>>    possible rather than eagerly creating new ones and (2) postponing the
>>    optimization of newly created ConvI2Ls. Remove "hook" node solution introduced
>>    in JDK-8217359 since this is subsumed by (2). Test that ConvI2LNode::Ideal() is
>>    called within iterative GVN using phase->is_IterGVN() rather than can_reshape,
>>    for clarity.
>>    
>>    Merge all tests into a single class. Reimplement the microbenchmark as a test
>>    case that should time out in case of a combinatorial explosion. Add a second
>>    similar microbenchmark that demonstrates the need for this generalization.
>>  - Merge master
>>  - 8254317: C2: Resource consumption of ConvI2LNode::Ideal() grows exponentially
>>    
>>    In the optimization ConvI2L(AddI(x, y)) -> AddL(ConvI2L(x), ConvI2L(y)) within
>>    ConvI2LNode::Ideal(), handle the special case x = y by feeding both inputs of
>>    AddL from a single ConvI2L node rather than creating two semantically equivalent
>>    ConvI2L nodes. This avoids an exponential number of calls to
>>    ConvI2LNode::Ideal() when dealing with long chains of AddI nodes. Disable the
>>    optimization for the pattern ConvI2L(SubI(x, x)), which is simplified to zero
>>    during parsing anyway. Add a set of regression tests for the transformation that
>>    cover different shapes of AddI subgraphs. Also add a microbenchmark that
>>    exercises the special case, for performance regression testing.
>
> test/hotspot/jtreg/compiler/conversions/TestMoveConvI2LThroughAddIs.java line 38:
> 
>> 36:  *          the short specified timeout.
>> 37:  * @library /test/lib /
>> 38:  * @run main/othervm -Xcomp -XX:-TieredCompilation -XX:-Inline
> 
> For future tests writing.
> In general we should avoid using -Xcomp because it does not provide profiling information to C2 and it may generate unexpected code because branches frequencies are not known.
> Preferable way is warm up test method by calling it in a loop enough times to make sure it is compiled and then verify results outside test method.
> Then you don't need -XX:CompileOnly command.

Without -Xcomp, the final code would not the same as expected, lots of branches have been optimized with profiling data which we **do** normally used. 

I wonder whether it's feasible to create such a test case that grows exponentially with profiling data. I think that may decriable the issue more realistic.

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

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