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

[Daniel Lundén]

On Mon, 3 Mar 2025 12:02:46 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update after Christian's review
>
> 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.

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

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