RFR: 8275330: C2: assert(n->is_Root() || n->is_Region() || n->is_Phi() || n->is_MachMerge() || def_block->dominates(block)) failed: uses must be dominated by definitions [v2]

[Christian Hagedorn]

On Thu, 25 Nov 2021 17:03:38 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> This is similar to previous bugs where:
>> 
>> - a cast/conv node captures a narrow type in a loop body because of a
>>   range check,
>> 
>> - the range check is optimized out of the loop, pre/main/post loop are
>>   created
>> 
>> - overunrolling causes the main loop to become unreachable (the range
>>   check, if still in the main loop, would fail), the cast transforms to
>>   top but c2 can't optimize the loop out
>> 
>> This was fixed by adding predicates above the main loop. With this
>> particular bug, the cast node is in the post loop. The fix I propose
>> is to also add predicates above the post loop. There are a few
>> locations in the code that cause a post loop to be added: either the
>> initial post loop or some other post loops for vectorization
>> support. I think the new predicates are needed in a all cases. To be
>> able to add predicates at these different points in the optimization
>> process, the new predicates are copied from the main loop predicates.
>> 
>> I also delayed folding of Opaque4 nodes to macro expansion rather than
>> post loop opts igvn. The reason for that is that I believe there's a
>> risk that an Opaque4 is removed (that is replaced by its input 2)
>> before its input 1 has a chance to constant fold. That wouldn't happen
>> with a debug build because we leave the tests in (that is replace the
>> Opaque4 node by its input 1) so that corner case is not properly
>> tested currently. The reason for leaving the tests in was to sanity
>> check that the tests are indeed correct.
>
> Roland Westrelin 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 three additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8275330
>  - reviews
>  - fix

Thanks for doing the updates, looks good!

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

Marked as reviewed by chagedorn (Reviewer).

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