RFR: 8267988: C2: assert(!addp->is_AddP() || addp->in(AddPNode::Base)->is_top() || addp->in(AddPNode::Base) == n->in(AddPNode::Base)) failed: Base pointers must match (addp 1301) [v3]

[Roland Westrelin]

On Thu, 10 Jun 2021 11:07:16 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:
>> 
>>  - review
>>  - merge
>>  - review
>>  - review
>>  - fix
>
> src/hotspot/share/opto/loopopts.cpp line 1510:
> 
>> 1508:           // is only added once both AddP nodes are sunk (see special code for AddP after the cast is created).
>> 1509:           if (x->in(0) == NULL && !x->is_DecodeNarrowPtr() &&
>> 1510:               !(x->is_AddP() && x->in(AddPNode::Address)->is_AddP() && x->in(AddPNode::Address)->in(AddPNode::Base) == x->in(AddPNode::Base))) {
> 
> I read the last check as "not a case of chained AddP with different bases". 
> Then the comparison should be `!=`, shouldn't it?

It's safe to add a cast if it's not a chained AddP with the same base otherwise there's a risk that adding a cast would make the base differ.

> src/hotspot/share/opto/loopopts.cpp line 1523:
> 
>> 1521:                 register_new_node(cast, x_ctrl);
>> 1522:                 x->replace_edge(in, cast);
>> 1523:                 if (x->is_AddP() && k == AddPNode::Base) {
> 
> Does the code assume that "inner" node is processed first?
> Otherwise, the "outer" node could be already processed and bases don't match anymore.
> 
> I'm asking because `PhaseIdealLoop::split_if_with_blocks()` (which calls `split_if_with_blocks_post()` and `try_sink_out_of_loop()` does process users first.

The expectation is that uses are processed first.  The:
!(x->is_AddP() && x->in(AddPNode::Address)->is_AddP() && x->in(AddPNode::Address)->in(AddPNode::Base) == x->in(AddPNode::Base))
check guarantees the "outer" node doesn't change the base when it's processed. When the "inner" node is processed, bases for the inner and outer node are updated.

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

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