RFR: 8347499: C2: Make `PhaseIdealLoop` eliminate more redundant safepoints in loops

[Emanuel Peter]

On Mon, 13 Jan 2025 01:12:20 GMT, Qizheng Xing <qxing at openjdk.org> wrote:

> In `PhaseIdealLoop`, `IdealLoopTree::check_safepts` method checks if any call that is guaranteed to have a safepoint dominates the tail of the loop. In the previous implementation, `check_safepts` would stop if it found a local non-call safepoint. At this time, if there was a call before the safepoint in the dom-path, this safepoint would not be eliminated.
> 
> <img width="353" alt="loop-safepoint" src="https://github.com/user-attachments/assets/c220e103-aaba-4e3f-98ac-1ddb6465c309" />
> 
> This patch changes the behavior of `check_safepts` to not stop when it finds a non-local safepoint. This makes simple loops with one method call ~3.8% faster (on aarch64).
> 
> 
> Benchmark                Mode  Cnt       Score      Error  Units
> LoopSafepoint.loopVar    avgt   15  208296.259 ± 1350.409  ns/op   # baseline
> LoopSafepoint.loopVar    avgt   15  200692.874 ±  616.770  ns/op   # this patch
> 
> 
> Testing: tier1-2 on x86_64 and aarch64.

It generally looks reasonable. Though I am struggling to understand all of the comments that are above `IdealLoopTree::check_safepts`, and their impact on your change. Specifically, what does this mean:

//  C) a loop is immune from an inner loop deleting a safepoint
//     if the loop has a call on the idom-path

There is also no explanation why. `if the loop has a call on the idom-path` - is that talking about a call in the outer loop, or the inner loop?

Another question: If we now removed safepoints in places where we would actually have needed them: how would we find out? I suppose we would get longer time to safepoint - higher latency in some cases. How would we catch this with our tests?

@rwestrel should probably have a look at this change, he knows more about this.

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

PR Review: https://git.openjdk.org/jdk/pull/23057#pullrequestreview-2558138385