RFR: JDK-8275854: C2: assert(stride_con != 0) failed: missed some peephole opt [v6]

[Tobias Hartmann]

On Tue, 2 Nov 2021 02:54:39 GMT, 王超 <duke at openjdk.java.net> wrote:

>> `If subsume` optimization will eliminate `LongCountedLoopEndNode` node by mistake, which will lead to `PhaseIdealLoop` optimization crash.
>> 
>> For example, the test of node 538 and node 553 will become the same after the first `PhaseIdealLoop` optimization. Node 555 is the back edge to the loop, and node 553 will be replaced by a `LongCountedLoopEndNode` node.
>> <img width="592" alt="image" src="https://user-images.githubusercontent.com/25214855/138656330-9f825c85-0468-40a5-9eb9-b576130648db.png">
>> 
>> 
>>  In the next `PhaseIdealLoop` optimization, node 538 find node 553 is redundant, and will subsume node 553. Then the `PhaseIdealLoop` optimization will crash, because there is no loop end node.
>> <img width="603" alt="image" src="https://user-images.githubusercontent.com/25214855/138656951-53685723-92c2-42d6-91ae-445107dcb01d.png">
>> 
>> There are two way to fix the crash, the first is like the way in this pr, just exit `IFNode subsume` optimization when it's a `LongCountedLoopEndNode` node. The second possible fix is that exchange the dominating `IF` node with the `LongCountedLoopEndNode` node:
>> 
>> diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp
>> index 38b40a6..31ff172 100644
>> --- a/src/hotspot/share/opto/ifnode.cpp
>> +++ b/src/hotspot/share/opto/ifnode.cpp
>> @@ -1674,6 +1674,21 @@ Node* IfNode::simple_subsuming(PhaseIterGVN* igvn) {
>>      }
>>    }
>>  
>> +  if (is_LongCountedLoopEnd()) {
>> +    set_req(0, dom->in(0));
>> +    set_req(1, dom->in(1));
>> +    dom->set_req(0, pre);
>> +    dom->set_req(1, igvn->intcon(is_always_true ? 1 : 0));
>> +    Node* proj0 = raw_out(0);
>> +    Node* proj1 = raw_out(1);
>> +    Node* dom_proj0 = dom->raw_out(0);
>> +    Node* dom_proj1 = dom->raw_out(1);
>> +    dom_proj0->set_req(0, this);
>> +    dom_proj1->set_req(0, this);
>> +    proj0->set_req(0, dom);
>> +    proj1->set_req(0, dom);
>> +  }
>> +
>>    if (bol->outcnt() == 0) {
>>      igvn->remove_dead_node(bol);    // Kill the BoolNode.
>>    }
>> diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp
>> index 6f7e34d..7955722 100644
>> --- a/src/hotspot/share/opto/loopnode.cpp
>> +++ b/src/hotspot/share/opto/loopnode.cpp
>> @@ -802,7 +802,7 @@ bool PhaseIdealLoop::transform_long_counted_loop(IdealLoopTree* loop, Node_List
>>    Node* back_control = head->in(LoopNode::LoopBackControl);
>>  
>>    // data nodes on back branch not supported
>> -  if (back_control->outcnt() > 1) {
>> +  if (back_control->outcnt() > 1 || back_control->Opcode() != Op_IfTrue) {
>>      return false;
>>    }
>
> 王超 has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Specify vm option needs option 'othervm'

That looks reasonable to me but please add a comment to the new code.

I'll run some testing in the meantime.

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

Marked as reviewed by thartmann (Reviewer).

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