Date: Sat, 27 May 2023 17:38:35 +0800 Subject: add 8305541-C2-Div-Mod-nodes-without-zero-check-could-be.patch --- src/hotspot/share/opto/loopnode.hpp | 3 + src/hotspot/share/opto/loopopts.cpp | 42 ++++- .../c2/TestSplitDivisionThroughPhi.java | 161 ++++++++++++++++++ 3 files changed, 205 insertions(+), 1 deletion(-) create mode 100644 test/hotspot/jtreg/compiler/c2/TestSplitDivisionThroughPhi.java diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index ebc3bd1db..0db6d0881 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1506,6 +1506,9 @@ private: void try_move_store_after_loop(Node* n); 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); + 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 // a magic (approximate) ratio derived from the equally magic constant 35000, diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index c0804ed67..9e0f1b2d2 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1999, 2023, 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 @@ -61,6 +61,10 @@ Node* PhaseIdealLoop::split_thru_phi(Node* n, Node* region, int policy) { return NULL; } + if (cannot_split_division(n, region)) { + return NULL; + } + // Bail out if 'n' is a Div or Mod node whose zero check was removed earlier (i.e. control is NULL) and its divisor is an induction variable // phi p of a trip-counted (integer) loop whose inputs could be zero (include zero in their type range). p could have a more precise type // range that does not necessarily include all values of its inputs. Since each of these inputs will be a divisor of the newly cloned nodes @@ -225,6 +229,42 @@ Node* PhaseIdealLoop::split_thru_phi(Node* n, Node* region, int policy) { return phi; } +// Return true if 'n' is a Div or Mod node (without zero check If node which was removed earlier) with a loop phi divisor +// of a trip-counted (integer or long) loop with a backedge input that could be zero (include zero in its type range). In +// this case, we cannot split the division to the backedge as it could freely float above the loop exit check resulting in +// a division by zero. This situation is possible because the type of an increment node of an iv phi (trip-counter) could +// include zero while the iv phi does not (see PhiNode::Value() for trip-counted loops where we improve types of iv phis). +// We also need to check other loop phis as they could have been created in the same split-if pass when applying +// PhaseIdealLoop::split_thru_phi() to split nodes through an iv phi. +bool PhaseIdealLoop::cannot_split_division(const Node* n, const Node* region) const { + const Type* zero; + switch (n->Opcode()) { + case Op_DivI: + case Op_ModI: + zero = TypeInt::ZERO; + break; + case Op_DivL: + case Op_ModL: + zero = TypeLong::ZERO; + break; + default: + return false; + } + + assert(n->in(0) == NULL, "divisions with zero check should already have bailed out earlier in split-if"); + Node* divisor = n->in(2); + return is_divisor_counted_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::loop_phi_backedge_type_contains_zero(const Node* phi_divisor, const Type* zero) const { + return _igvn.type(phi_divisor->in(LoopNode::LoopBackControl))->filter_speculative(zero) != Type::TOP; +} + //------------------------------dominated_by------------------------------------ // Replace the dominated test with an obvious true or false. Place it on the // IGVN worklist for later cleanup. Move control-dependent data Nodes on the diff --git a/test/hotspot/jtreg/compiler/c2/TestSplitDivisionThroughPhi.java b/test/hotspot/jtreg/compiler/c2/TestSplitDivisionThroughPhi.java new file mode 100644 index 000000000..5a42e7d36 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/TestSplitDivisionThroughPhi.java @@ -0,0 +1,161 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, Huawei Technologies Co., Ltd. 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** +* @test +* @key stress randomness +* @bug 8299259 +* @requires vm.compiler2.enabled +* @summary Test various cases of divisions/modulo which should not be split through iv phis. +* @run main/othervm -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:LoopUnrollLimit=0 -XX:+StressGCM -XX:StressSeed=884154126 +* -XX:CompileCommand=compileonly,compiler.splitif.TestSplitDivisionThroughPhi::* +* compiler.splitif.TestSplitDivisionThroughPhi +*/ + +/** +* @test +* @key stress randomness +* @bug 8299259 +* @requires vm.compiler2.enabled +* @summary Test various cases of divisions/modulo which should not be split through iv phis. +* @run main/othervm -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:LoopUnrollLimit=0 -XX:+StressGCM +* -XX:CompileCommand=compileonly,compiler.splitif.TestSplitDivisionThroughPhi::* +* compiler.splitif.TestSplitDivisionThroughPhi +*/ + +package compiler.splitif; + +public class TestSplitDivisionThroughPhi { + static int iFld; + static long lFld; + static boolean flag; + + + public static void main(String[] strArr) { + for (int i = 0; i < 5000; i++) { + testPushDivIThruPhi(); + testPushDivIThruPhiInChain(); + testPushModIThruPhi(); + testPushModIThruPhiInChain(); + testPushDivLThruPhi(); + testPushDivLThruPhiInChain(); + testPushModLThruPhi(); + testPushModLThruPhiInChain(); + } + } + + // Already fixed by JDK-8248552. + static void testPushDivIThruPhi() { + for (int i = 10; i > 1; i -= 2) { + // The Div node is only split in later loop opts phase because the zero divisor check is only removed + // in IGVN after the first loop opts phase. + // + // iv phi i type: [2..10] + // When splitting the DivI through the iv phi, it ends up on the back edge with the trip count decrement + // as input which has type [0..8]. We end up executing a division by zero on the last iteration because + // the DivI it is not pinned to the loop exit test and can freely float above the loop exit check. + 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() { + for (int i = 10; i > 1; i -= 2) { + // Empty one iteration loop which is only removed after split if in first loop opts phase. This prevents + // that the Mul node is already split through the iv phi while the Div node cannot be split yet due to + // the zero divisor check which can only be removed in the IGVN after the first loop opts pass. + for (int j = 0; j < 1; j++) { + } + iFld = 10 / (i * 100); + } + } + + // Already fixed by JDK-8248552. + static void testPushModIThruPhi() { + for (int i = 10; i > 1; i -= 2) { + iFld = 10 / i; + } + } + + // Same as above but with ModI. + static void testPushModIThruPhiInChain() { + for (int i = 10; i > 1; i -= 2) { + for (int j = 0; j < 1; j++) { + } + iFld = 10 / (i * 100); + } + } + + // Long cases only trigger since JDK-8256655. + + // Same as above but with DivL. + static void testPushDivLThruPhi() { + for (long i = 10; i > 1; i -= 2) { + lFld = 10L / i; + + // Loop that is not removed such that we do not transform the outer LongCountedLoop (only done if innermost) + for (int j = 0; j < 10; j++) { + flag = !flag; + } + } + } + + // Same as above but with DivL. + static void testPushDivLThruPhiInChain() { + for (long i = 10; i > 1; i -= 2) { + for (int j = 0; j < 1; j++) { + } + lFld = 10L / (i * 100L); + + for (int j = 0; j < 10; j++) { + flag = !flag; + } + } + } + + // Same as above but with ModL + static void testPushModLThruPhi() { + for (long i = 10; i > 1; i -= 2) { + lFld = 10L % i; + + for (int j = 0; j < 10; j++) { + flag = !flag; + } + } + } + + // Same as above but with ModL + static void testPushModLThruPhiInChain() { + for (long i = 10; i > 1; i -= 2) { + for (int j = 0; j < 1; j++) { + } + lFld = 10L % (i * 100L); + + for (int j = 0; j < 10; j++) { + flag = !flag; + } + } + } +} -- 2.22.0