[jdk16] RFR: 8258393: Shenandoah: "graph should be schedulable" assert failure [v2]

[Tobias Hartmann]

On Tue, 5 Jan 2021 13:00:10 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> This is a shenandoah bug but the fix is in shared code.
>> 
>> At barrier expansion time, we need the raw memory state for the
>> control at which the barrier is expanded. Before expansion, the memory
>> graph is traversed and memory state for each control is recorded. In
>> that process, if opportunities to improve the memory graph are found,
>> the graph is modified. In the case of the failure, a raw memory load
>> was assigned a call projection as control but for that control, no
>> memory state was recorded and the fall back is to use the call's input
>> memory state. As a consequence the load's memory input is updated to a
>> wrong memory state. This causes the assert failure.
>> 
>> The call has 2 memory projections: one for the fallthrough and one for
>> the exception path. One projection is the memory state for the
>> CatchProj for the fallthrough, the other one for the CatchProj of the
>> exception path. For the control projection out of the call, there's no
>> memory state that makes sense and assigning the control projection of
>> the call as control for the load is the root cause of the
>> problem. This happens because anti-dependence analysis is too
>> conservative: a memory Phi that merges states from the exception and
>> fallthrough path is considered a dependency and that pushed the load
>> right after the call.
>> 
>> I propose to be less conservative in anti-dependence analysis for
>> Phis. For a Phi, when computing the LCA, I think it's sufficient to
>> only consider region's inputs that we actually reach by following the
>> memory edges and that's what I propose here.
>
> 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 two additional commits since the last revision:
> 
>  - review
>  - test & fix

Thanks for updating, looks good to me.

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

Marked as reviewed by thartmann (Reviewer).

PR: https://git.openjdk.java.net/jdk16/pull/41