RFR: 8265132 : C2 compilation fails with assert "missing precedence edge" [v4]

[Vladimir Kozlov]

On Thu, 27 May 2021 21:05:33 GMT, Jamsheed Mohammed C M <jcm at openjdk.org> wrote:

>> Issue is similar to https://bugs.openjdk.java.net/browse/JDK-8261730
>> but happens at next https://github.com/jamsheedcm/jdk/blob/master/src/hotspot/share/opto/gcm.cpp#L830
>> 
>> Request for review
>
> Jamsheed Mohammed C M has updated the pull request incrementally with one additional commit since the last revision:
> 
>   improving the fix for raise_LCA_above_marks

I prefer version 02 to prevent `must_raise_LCA` in our edge case. Otherwise you affect code for `store->is_Phi()`.  And it is more complicated.
Yes we would need to execute the check in product version too.
First, please don't do that for general (multi-blocks) case. Do it only for implicit_null_check and uncommon trap blocks.

Block* load_block = get_block_for_node(load);
Node* end = store_block->end();
if (end->is_MachNullCheck() && (end->in(1) == store) && store_block->dominates(load_block) ) {
     Node* if_true = end->find_out_with(Op_IfTrue);
     assert(if_true != NULL, "null check without null projection");
     Node* null_block_region = if_true->find_out_with(Op_Region);
     assert(null_block_region != NULL, "null check without null region");
     return get_block_for_node(null_block_region) == load_block;
}

src/hotspot/share/opto/gcm.cpp line 548:

> 546: // This function is used by insert_anti_dependences to find unrelated loads
> 547: // stores(but aliases into same) in non-null, null blocks.
> 548: // and for the same reasons it doesn't requires an anti-dependence edge.

Confusing 3rd line in comment. And first line could be simple:

This function is used by insert_anti_dependences to find unrelated loads for stores in implicit null checks.

src/hotspot/share/opto/gcm.cpp line 554:

> 552:   // perform an implicit null check, and 'load' is placed in the null
> 553:   // block. In this case it is safe to ignore the anti-dependence, as the
> 554:   // null block is only reached if 'store' tries to write to null.

May add additional comment line:

// block. In this case it is safe to ignore the anti-dependence, as the
// null block is only reached if 'store' tries to write to null object and
// 'load' read from non-null object (there is preceding check for that)
// These objects can't be the same.

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

Changes requested by kvn (Reviewer).

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