Skip to content

Commit

Permalink
8336729: C2: Div/Mod nodes without zero check could be split through …
Browse files Browse the repository at this point in the history
…iv phi of outer loop of long counted loop nest resulting in SIGFPE
  • Loading branch information
chhagedorn committed Aug 15, 2024
1 parent 4669e7b commit 8ba7417
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/loopnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1578,7 +1578,7 @@ class PhaseIdealLoop : public PhaseTransform {
bool identical_backtoback_ifs(Node *n);
bool can_split_if(Node *n_ctrl);
bool cannot_split_division(const Node* n, const Node* region) const;
static bool is_divisor_counted_loop_phi(const Node* divisor, const Node* loop);
static bool is_divisor_loop_phi(const Node* divisor, const Node* loop);
bool loop_phi_backedge_type_contains_zero(const Node* phi_divisor, const Type* zero) const;

// Determine if a method is too big for a/another round of split-if, based on
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/opto/loopopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,12 +295,12 @@ bool PhaseIdealLoop::cannot_split_division(const Node* n, const Node* region) co
}

Node* divisor = n->in(2);
return is_divisor_counted_loop_phi(divisor, region) &&
return is_divisor_loop_phi(divisor, region) &&
loop_phi_backedge_type_contains_zero(divisor, zero);
}

bool PhaseIdealLoop::is_divisor_counted_loop_phi(const Node* divisor, const Node* loop) {
return loop->is_BaseCountedLoop() && divisor->is_Phi() && divisor->in(0) == loop;
bool PhaseIdealLoop::is_divisor_loop_phi(const Node* divisor, const Node* loop) {
return loop->is_Loop() && divisor->is_Phi() && divisor->in(0) == loop;
}

bool PhaseIdealLoop::loop_phi_backedge_type_contains_zero(const Node* phi_divisor, const Type* zero) const {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -43,6 +43,28 @@
* compiler.splitif.TestSplitDivisionThroughPhi
*/

/**
* @test
* @key stress randomness
* @bug 8336729
* @requires vm.compiler2.enabled
* @summary Test various cases of divisions/modulo which should not be split through iv phis.
* @run main/othervm -Xcomp -XX:+UnlockDiagnosticVMOptions -XX:LoopUnrollLimit=0 -XX:+StressGCM -XX:StressSeed=3434
* -XX:CompileCommand=compileonly,compiler.splitif.TestSplitDivisionThroughPhi::*
* compiler.splitif.TestSplitDivisionThroughPhi
*/

/**
* @test
* @key stress randomness
* @bug 8336729
* @requires vm.compiler2.enabled
* @summary Test various cases of divisions/modulo which should not be split through iv phis.
* @run main/othervm -Xcomp -XX:+UnlockDiagnosticVMOptions -XX:LoopUnrollLimit=0 -XX:+StressGCM
* -XX:CompileCommand=compileonly,compiler.splitif.TestSplitDivisionThroughPhi::*
* compiler.splitif.TestSplitDivisionThroughPhi
*/

package compiler.splitif;

public class TestSplitDivisionThroughPhi {
Expand All @@ -61,6 +83,8 @@ public static void main(String[] strArr) {
testPushDivLThruPhiInChain();
testPushModLThruPhi();
testPushModLThruPhiInChain();
testPushDivLThruPhiForOuterLongLoop();
testPushModLThruPhiForOuterLongLoop();
}
}

Expand All @@ -78,6 +102,27 @@ static void testPushDivIThruPhi() {
}
}

// Fixed with JDK-8336792.
static void testPushDivLThruPhiForOuterLongLoop() {
// This loop is first transformed into a LongCountedLoop in the first loop opts phase.
// In the second loop opts phase, the LongCountedLoop is split into an inner and an outer loop. Both get the
// same iv phi type which is [2..10]. Only the inner loop is transformed into a CountedLoopNode while the outer
// loop is still a LoopNode. We run into the same problem as described in testPushDivIThruPhi() when splitting
// the DivL node through the long iv phi of the outer LoopNode.
// The fix for JDK-8299259 only prevents this splitting for CountedLoopNodes. We now extend it to LoopNodes
// in general.
for (long i = 10; i > 1; i -= 2) {
lFld = 10 / i;
}
}

// Same as testPushDivLThruPhiForOuterLongLoop() but for ModL.
static void testPushModLThruPhiForOuterLongLoop() {
for (int i = 10; i > 1; i -= 2) {
iFld = 10 % i;
}
}

// Same as above but with an additional Mul node between the iv phi and the Div node. Both nodes are split through
// the iv phi in one pass of Split If.
static void testPushDivIThruPhiInChain() {
Expand Down

0 comments on commit 8ba7417

Please sign in to comment.