RFR: 8282045: When loop strip mining fails, safepoints are removed from loop anyway

[Tobias Hartmann]

On Thu, 17 Feb 2022 10:00:32 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> I noticed that if loop strip mining fails because a safepoint is not
> found right above the exit test (following partial peel for instance),
> all safepoints are removed from the loop anyway. That's fixed by the
> change in IdealLoopTree::counted_loop() where rather than test if loop
> strip mining is enabled, the check now verifies that loop strip mining
> was successful.
> 
> With that change,
> compiler/c2/irTests/TestAutoVectorization2DArray.java fails. The loop
> is not converted into a strip mined loop because there's no safepoint
> above the exit test after partial peeling. The loop strip mining logic
> is too strict when it comes to the safepoint location. Any safepoint
> that dominates the exit and is in the loop as long as there's no side
> effect between the safepoint and the exit can be used. The patch
> implements that change as well. TestAutoVectorization2DArray.java
> passes as a result.
> 
> The existing requirement to have no safepoint on the backedge is too
> strict as well. If the loop has another safepoint that can be used for
> strip mining, then the safepoint on the backedge can safely be
> dropped. That's also implemented by the patch.

Noticed a typo in `PhaseIdealLoop::find_safepoint` that you might want to fix as well with this patch:
`there's not side effect` -> `there's no side effect`

src/hotspot/share/opto/loopnode.cpp line 1852:

> 1850:   }
> 1851: 
> 1852:   Node *sfpt2 = NULL;

Suggestion:

  Node* sfpt2 = NULL;

src/hotspot/share/opto/loopnode.cpp line 1863:

> 1861: 
> 1862:   if (x->in(LoopNode::LoopBackControl)->Opcode() == Op_SafePoint) {
> 1863:     Node *sfpt = x->in(LoopNode::LoopBackControl);

Suggestion:

    Node* sfpt = x->in(LoopNode::LoopBackControl);

src/hotspot/share/opto/loopnode.cpp line 1874:

> 1872:       return false;
> 1873:     }
> 1874:     if (deleteable) {

Can't you check `is_deleteable_safept` inline here like old code did?

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

PR: https://git.openjdk.java.net/jdk/pull/7513