diff --git a/src/hotspot/share/opto/castnode.cpp b/src/hotspot/share/opto/castnode.cpp index a93c9b382f9..1771107c851 100644 --- a/src/hotspot/share/opto/castnode.cpp +++ b/src/hotspot/share/opto/castnode.cpp @@ -213,13 +213,7 @@ const Type* CastIINode::Value(PhaseGVN* phase) const { // Similar to ConvI2LNode::Value() for the same reasons // see if we can remove type assertion after loop opts - // But here we have to pay extra attention: - // Do not narrow the type of range check dependent CastIINodes to - // avoid corruption of the graph if a CastII is replaced by TOP but - // the corresponding range check is not removed. - if (!_range_check_dependency) { - res = widen_type(phase, res, T_INT); - } + res = widen_type(phase, res, T_INT); return res; } @@ -239,11 +233,11 @@ Node *CastIINode::Ideal(PhaseGVN *phase, bool can_reshape) { if (progress != nullptr) { return progress; } - if (can_reshape && !_range_check_dependency && !phase->C->post_loop_opts_phase()) { + if (can_reshape && !phase->C->post_loop_opts_phase()) { // makes sure we run ::Value to potentially remove type assertion after loop opts phase->C->record_for_post_loop_opts_igvn(this); } - if (!_range_check_dependency) { + if (!_type->is_int()->empty()) { return optimize_integer_cast(phase, T_INT); } return nullptr; @@ -254,13 +248,6 @@ Node* CastIINode::Identity(PhaseGVN* phase) { if (progress != this) { return progress; } - if (_range_check_dependency) { - if (phase->C->post_loop_opts_phase()) { - return this->in(1); - } else { - phase->C->record_for_post_loop_opts_igvn(this); - } - } return this; } diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index 25c7ced8aeb..445f22c6b80 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -3464,6 +3464,10 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f } break; } + case Op_CastII: { + remove_range_check_cast(n->as_CastII()); + } + break; #ifdef _LP64 case Op_CmpP: // Do this transformation here to preserve CmpPNode::sub() and @@ -3615,16 +3619,6 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f #endif -#ifdef ASSERT - case Op_CastII: - // Verify that all range check dependent CastII nodes were removed. - if (n->isa_CastII()->has_range_check()) { - n->dump(3); - assert(false, "Range check dependent CastII node was not removed"); - } - break; -#endif - case Op_ModI: if (UseDivMod) { // Check if a%b and a/b both exist @@ -3633,6 +3627,8 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f // Replace them with a fused divmod if supported if (Matcher::has_match_rule(Op_DivModI)) { DivModINode* divmod = DivModINode::make(n); + divmod->add_prec_from(n); + divmod->add_prec_from(d); d->subsume_by(divmod->div_proj(), this); n->subsume_by(divmod->mod_proj(), this); } else { @@ -3653,6 +3649,8 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f // Replace them with a fused divmod if supported if (Matcher::has_match_rule(Op_DivModL)) { DivModLNode* divmod = DivModLNode::make(n); + divmod->add_prec_from(n); + divmod->add_prec_from(d); d->subsume_by(divmod->div_proj(), this); n->subsume_by(divmod->mod_proj(), this); } else { @@ -3673,6 +3671,8 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f // Replace them with a fused unsigned divmod if supported if (Matcher::has_match_rule(Op_UDivModI)) { UDivModINode* divmod = UDivModINode::make(n); + divmod->add_prec_from(n); + divmod->add_prec_from(d); d->subsume_by(divmod->div_proj(), this); n->subsume_by(divmod->mod_proj(), this); } else { @@ -3693,6 +3693,8 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f // Replace them with a fused unsigned divmod if supported if (Matcher::has_match_rule(Op_UDivModL)) { UDivModLNode* divmod = UDivModLNode::make(n); + divmod->add_prec_from(n); + divmod->add_prec_from(d); d->subsume_by(divmod->div_proj(), this); n->subsume_by(divmod->mod_proj(), this); } else { @@ -3894,6 +3896,34 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f } } +void Compile::remove_range_check_cast(CastIINode* cast) { + if (cast->has_range_check()) { + // Range check CastII nodes feed into an address computation subgraph. Remove them to let that subgraph float freely. + // For memory access or integer divisions nodes that depend on the cast, record the dependency on the cast's control + // as a precedence edge, so they can't float above the cast in case that cast's narrowed type helped eliminate a + // range check or a null divisor check. + assert(cast->in(0) != nullptr, "All RangeCheck CastII must have a control dependency"); + ResourceMark rm; + Unique_Node_List wq; + wq.push(cast); + for (uint next = 0; next < wq.size(); ++next) { + Node* m = wq.at(next); + for (DUIterator_Fast imax, i = m->fast_outs(imax); i < imax; i++) { + Node* use = m->fast_out(i); + if (use->is_Mem() || use->is_div_or_mod(T_INT) || use->is_div_or_mod(T_LONG)) { + use->ensure_control_or_add_prec(cast->in(0)); + } else if (!use->is_CFG() && !use->is_Phi()) { + wq.push(use); + } + } + } + cast->subsume_by(cast->in(1), this); + if (cast->outcnt() == 0) { + cast->disconnect_inputs(this); + } + } +} + //------------------------------final_graph_reshaping_walk--------------------- // Replacing Opaque nodes with their input in final_graph_reshaping_impl(), // requires that the walk visits a node's inputs before visiting the node. diff --git a/src/hotspot/share/opto/compile.hpp b/src/hotspot/share/opto/compile.hpp index e1d9b61f7f8..9a873b8b9cb 100644 --- a/src/hotspot/share/opto/compile.hpp +++ b/src/hotspot/share/opto/compile.hpp @@ -53,6 +53,7 @@ class Block; class Bundle; class CallGenerator; class CallStaticJavaNode; +class CastIINode; class CloneMap; class CompilationFailureInfo; class ConnectionGraph; @@ -1314,6 +1315,8 @@ private: BasicType out_bt, BasicType in_bt); static Node* narrow_value(BasicType bt, Node* value, const Type* type, PhaseGVN* phase, bool transform_res); + + void remove_range_check_cast(CastIINode* cast); }; #endif // SHARE_OPTO_COMPILE_HPP diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp index fa85344d0da..cadea46e2c9 100644 --- a/src/hotspot/share/opto/node.cpp +++ b/src/hotspot/share/opto/node.cpp @@ -2878,6 +2878,15 @@ void Node::ensure_control_or_add_prec(Node* c) { } } +void Node::add_prec_from(Node* n) { + for (uint i = n->req(); i < n->len(); i++) { + Node* prec = n->in(i); + if (prec != nullptr) { + add_prec(prec); + } + } +} + bool Node::is_dead_loop_safe() const { if (is_Phi()) { return true; @@ -2901,6 +2910,9 @@ bool Node::is_dead_loop_safe() const { return false; } +bool Node::is_div_or_mod(BasicType bt) const { return Opcode() == Op_Div(bt) || Opcode() == Op_Mod(bt) || + Opcode() == Op_UDiv(bt) || Opcode() == Op_UMod(bt); } + //============================================================================= //------------------------------yank------------------------------------------- // Find and remove diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp index 04631dcbae2..412e193a612 100644 --- a/src/hotspot/share/opto/node.hpp +++ b/src/hotspot/share/opto/node.hpp @@ -1143,6 +1143,7 @@ public: // Set control or add control as precedence edge void ensure_control_or_add_prec(Node* c); + void add_prec_from(Node* n); // Visit boundary uses of the node and apply a callback function for each. // Recursively traverse uses, stopping and applying the callback when @@ -1254,6 +1255,8 @@ public: // Whether this is a memory phi node bool is_memory_phi() const { return is_Phi() && bottom_type() == Type::MEMORY; } + bool is_div_or_mod(BasicType bt) const; + //----------------- Printing, etc #ifndef PRODUCT public: @@ -2023,6 +2026,10 @@ Op_IL(URShift) Op_IL(LShift) Op_IL(Xor) Op_IL(Cmp) +Op_IL(Div) +Op_IL(Mod) +Op_IL(UDiv) +Op_IL(UMod) inline int Op_ConIL(BasicType bt) { assert(bt == T_INT || bt == T_LONG, "only for int or longs"); diff --git a/test/hotspot/jtreg/compiler/rangechecks/TestArrayAccessAboveRCAfterRCCastIIEliminated.java b/test/hotspot/jtreg/compiler/rangechecks/TestArrayAccessAboveRCAfterRCCastIIEliminated.java new file mode 100644 index 00000000000..7f22db08a36 --- /dev/null +++ b/test/hotspot/jtreg/compiler/rangechecks/TestArrayAccessAboveRCAfterRCCastIIEliminated.java @@ -0,0 +1,474 @@ +/* + * Copyright (c) 2024, Red Hat, Inc. 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 + * @bug 8324517 + * @summary C2: out of bound array load because of dependency on removed range check CastIIs + * + * @run main/othervm -XX:-TieredCompilation -XX:-UseOnStackReplacement -XX:-BackgroundCompilation + * -XX:CompileCommand=dontinline,TestArrayAccessAboveRCAfterRCCastIIEliminated::notInlined + * TestArrayAccessAboveRCAfterRCCastIIEliminated + * @run main/othervm -XX:-TieredCompilation -XX:-UseOnStackReplacement -XX:-BackgroundCompilation + * -XX:CompileCommand=dontinline,TestArrayAccessAboveRCAfterRCCastIIEliminated::notInlined + * -XX:+UnlockDiagnosticVMOptions -XX:+StressGCM TestArrayAccessAboveRCAfterRCCastIIEliminated + * @run main/othervm TestArrayAccessAboveRCAfterRCCastIIEliminated + * @run main/othervm -XX:CompileCommand=dontinline,TestArrayAccessAboveRCAfterRCCastIIEliminated::notInlined + * TestArrayAccessAboveRCAfterRCCastIIEliminated + * + */ + +public class TestArrayAccessAboveRCAfterRCCastIIEliminated { + private static int intField; + private static long longField; + private static volatile int volatileField; + + public static void main(String[] args) { + int[] array = new int[100]; + for (int i = 0; i < 20_000; i++) { + test1(9, 10, 1, true); + test1(9, 10, 1, false); + test2(9, 10, 1, true); + test2(9, 10, 1, false); + test3(9, 10, 1, true); + test3(9, 10, 1, false); + test4(9, 10, 1, true); + test4(9, 10, 1, false); + test5(9, 10, 1, true); + test5(9, 10, 1, false); + test6(9, 10, 1, true); + test6(9, 10, 1, false); + test7(9, 10, 1, true); + test7(9, 10, 1, false); + test8(9, 10, 1, true); + test8(9, 10, 1, false); + test9(9, 10, 1, true); + test9(9, 10, 1, false); + test10(9, 10, 1, true); + test10(9, 10, 1, false); + test11(9, 10, 1, true); + test11(9, 10, 1, false); + test12(9, 10, 1, true); + test12(9, 10, 1, false); + test13(9, 10, 1, true); + test13(9, 10, 1, false); + } + try { + test1(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test2(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test3(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test4(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test5(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test6(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test7(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test8(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test9(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test10(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test11(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test12(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + try { + test13(-1, 10, 1, true); + } catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) { + } + } + + private static void test1(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = array[otherArray.length]; + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = array[otherArray.length]; + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test2(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = 1 / (otherArray.length + 1); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = 1 / (otherArray.length + 1); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test3(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = 1L / (otherArray.length + 1); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = 1L / (otherArray.length + 1); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test4(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = 1 % (otherArray.length + 1); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = 1 % (otherArray.length + 1); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test5(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = 1L % (otherArray.length + 1); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = 1L % (otherArray.length + 1); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test6(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = 1 % (otherArray.length + 1) + 1 / (otherArray.length + 1); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = 1 % (otherArray.length + 1) + 1 / (otherArray.length + 1); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test7(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = 1L % (otherArray.length + 1) + 1L / (otherArray.length + 1); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = 1L % (otherArray.length + 1) + 1L / (otherArray.length + 1); + } + for (int k = 0; k < 10; k++) { + + } + } + private static void test8(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = Integer.divideUnsigned(1, (otherArray.length + 1)); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = Integer.divideUnsigned(1, (otherArray.length + 1)); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test9(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = Long.divideUnsigned(1L, (otherArray.length + 1)); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = Long.divideUnsigned(1L, (otherArray.length + 1)); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test10(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = Integer.remainderUnsigned(1, (otherArray.length + 1)); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = Integer.remainderUnsigned(1, (otherArray.length + 1)); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test11(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = Long.remainderUnsigned(1L, (otherArray.length + 1)); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = Long.remainderUnsigned(1L, (otherArray.length + 1)); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test12(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = Integer.divideUnsigned(1, (otherArray.length + 1)) + + Integer.remainderUnsigned(1, (otherArray.length + 1)); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + intField = Integer.divideUnsigned(1, (otherArray.length + 1)) + + Integer.remainderUnsigned(1, (otherArray.length + 1)); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void test13(int i, int j, int flag, boolean flag2) { + i = Math.min(i, 9); + int[] array = new int[10]; + notInlined(array); + if (flag == 0) { + } + if (flag2) { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = Long.remainderUnsigned(1L, (otherArray.length + 1)) + + Long.divideUnsigned(1L, (otherArray.length + 1)); + } else { + float[] newArray = new float[j]; + newArray[i] = 42; + float[] otherArray = new float[i]; + if (flag == 0) { + } + longField = Long.remainderUnsigned(1L, (otherArray.length + 1)) + + Long.divideUnsigned(1L, (otherArray.length + 1)); + } + for (int k = 0; k < 10; k++) { + + } + } + + private static void notInlined(int[] array) { + + } +}