RFR: 8333393: PhaseCFG::insert_anti_dependences can fail to raise LCAs and to add necessary anti-dependence edges [v2]

[Emanuel Peter]

On Thu, 9 Jan 2025 09:25:31 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> My understanding is that anti-dependence edges are only relevant for local scheduling (within blocks). Because Phis merge control-flow paths (by definition at the start of blocks), I would say it makes little sense to add an anti-dependence edge to Phi nodes. Does it make sense semantically to schedule loads before Phi nodes within a block? I don't think so, but I may be wrong.
>> 
>> I think what you are getting at is, at the block level, whether or not it is possible to raise the LCA above the Phi itself, rather than before the relevant inputs. That would make the scheduling less conservative. Have a look at [this comment](https://github.com/openjdk/jdk/pull/22852/files#diff-13dc4f80ba6ccaa27b0612318074e35200ffe9314405e30ace331807e56b5f60L870-L876) in the source. There are previous attempts at making the LCA raising less conservative in this manner (see, e.g., [JDK-8192992](https://bugs.openjdk.org/browse/JDK-8192992)), but it turns out to be quite tricky to get right. It is definitely an issue separate from the present one!
>
> Hmm, ok. I think the description here should be made more clear then, and explain what the strategy is, and attempt a kind of informal proof why this is an ok approach. What do you think?
> 
> I mean your comment says there are other cases, but it does not really reassure me that we have all cases covered 😅

Another naming question:
`other relevant initial memory states`
What does the `initial` mean to you? To me, it is just the memory state of the load. `initial` only because we also look at other memory states.

If my definition is correct, then I wonder if talking about "other initial memory states" makes sense, or if they should have a different name for clarity. Maybe `root_memory_state` or alike. That would make sense: We have multiple trees, starting at `root_memory_state` each. `initial_mem` is one of them. `initial` only refers to things in the initial block, where the `initial_mem` is located.

Maybe you have a different definition - but it would be good to have a clear one stated in the comments.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22852#discussion_r1908724253