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

[Emanuel Peter]

On Mon, 3 Mar 2025 13:18:42 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:

>> src/hotspot/share/opto/cfgnode.cpp line 2062:
>> 
>>> 2060: // PhiNode::Ideal. This is really a const method, but Node_List currently only
>>> 2061: // permits non-const elements.
>>> 2062: bool PhiNode::is_split_through_mergemem_terminating() {
>> 
>> This method could walk a significant part of the graph, right? And since this happens during IGVN, this could happen repeatedly, correct?
>> 
>> Often we have limits on traversals, but maybe we don't want that here.
>> 
>> I'm just wondering if this could have an impact on compile time. But then again: I don't know if there is even an alternative 🙈
>
> Yes, in theory it could walk a significant part of the graph. However, it just walks bot `MergeMem`s and `Phi`s, and stops at actual defining stores. With an assumption that the defining stores of `Phi`s are generally close to the `Phi`, it should run fairly quick in practice. Given that we now know that this idealization is needed for soundness, adding a limit sounds dangerous. But, I guess we could add a limit after which we bailout (and perhaps assert).
> 
> I ran C2 compilation speed testing on DaCapo and didn't see any statistically significant regression (although there's quite a bit of noise). Just looking at the mean compilation time across all benchmarks, there is a slight regression. On Linux x64 (which is the least noisy), there was a 0.84% mean regression in C2 compilation time. I could also run some more experiments.

I think that sounds reasonable. Not sure if more experiments are warranted now, I'm happy with the your due diligence as you report above ;)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23691#discussion_r1977517241