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

[Christian Hagedorn]

Thank you for your review Vladimir!

Best regards,
Christian

On 05.12.19 14:31, Vladimir Ivanov wrote:
> Looks good.
> 
> Best regards,
> Vladimir Ivanov
> 
> On 05.12.2019 16: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 
>>