[14] RFR(S): 8229994: assert(false) failed: Bad graph detected in get_early_ctrl_for_expensive

[Christian Hagedorn]

Thank you Tobias for your review! I fixed it.

Best regards,
Christian

On 05.12.19 16:16, Tobias Hartmann wrote:
> Hi Christian,
> 
> looks good to me but the CompileCommand argument in the @run statement of the test should come
> before the test class name (no new webrev required).
> 
> Best regards,
> Tobias
> 
> On 05.12.19 14:22, Christian Hagedorn wrote:
>> Hi
>>
>> Please review the following patch:
>> https://bugs.openjdk.java.net/browse/JDK-8229994
>> http://cr.openjdk.java.net/~chagedorn/8229994/webrev.00/
>>
>> As a part of loop peeling, loop-invariant dominating tests are moved out of the loop [1]. Doing so
>> requires to change all control inputs coming from an independent test 'test' to data nodes inside
>> the loop body to use the peeled version 'peeled_test' of 'test' instead [2]: the control test->data
>> will be changed to peeled_test->data. If a data node is expensive then an assertion [3] checks the
>> dominance relation of 'test' and 'peeled_test' ('peeled_test' should dominate 'test') but fails to
>> find 'peeled_test' as part of the idom chain starting from the previous control input 'test' of that
>> expensive node.
>>
>> The reason is that the idom information was not correctly set before at [4] after creating the
>> peeled nodes. If 'head' (a CountedLoop node) has another OuterStripMinedLoop node, let's say
>> 'outer_head', as an input then 'outer_head' is set as idom of 'head' instead of setting the idom of
>> 'outer_head' to the new loop entry from the peeled iteration. We then miss all the idom information
>> of the peeled iteration and the idom of 'outer_head' still points to the old loop entry node before
>> peeling.
>>
>> The fix is straight forward to also account for loop strip mined loops when correcting the idom to
>> the new loop entry from the peeled iteration at [4].
>>
>> Thank you!
>>
>> Best regards,
>> Christian
>>
>>
>> [1] http://hg.openjdk.java.net/jdk/jdk/file/99b71c5b02ff/src/hotspot/share/opto/loopTransform.cpp#l669
>> [2] http://hg.openjdk.java.net/jdk/jdk/file/99b71c5b02ff/src/hotspot/share/opto/loopopts.cpp#l271
>> [3] http://hg.openjdk.java.net/jdk/jdk/file/99b71c5b02ff/src/hotspot/share/opto/loopnode.cpp#l153
>> [4] http://hg.openjdk.java.net/jdk/jdk/file/99b71c5b02ff/src/hotspot/share/opto/loopTransform.cpp#l658