From adcb64323c7fb4a2641482c5eb3150b1bb37309b Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Mon, 8 Jan 2024 23:56:57 +0100 Subject: [PATCH 01/16] 8323220: Reassociate loop invariants involved in Cmps and Add/Subs --- src/hotspot/share/opto/loopTransform.cpp | 57 ++++- src/hotspot/share/opto/loopnode.hpp | 4 +- .../InvariantCodeMotionReassociateCmp.java | 221 ++++++++++++++++++ 3 files changed, 270 insertions(+), 12 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 3d3e7bdaa7c7b..771220472a567 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -263,6 +263,25 @@ int IdealLoopTree::find_invariant(Node* n, PhaseIdealLoop *phase) { return 0; } +//---------------------is_associative_cmp------------------------- +// Return TRUE if "n" is an associative cmp node. A cmp node is +// associative if it is only used for equals or not-equals +// comparisons of integers or longs. We cannot reassociate +// non-equality comparisons due to possibility of overflow. +bool IdealLoopTree::is_associative_cmp(Node* n) { + if (n->Opcode() != Op_CmpI && n->Opcode() != Op_CmpL) { + return false; + } + for (DUIterator i = n->outs(); n->has_out(i); i++) { + BoolNode *boolOut = n->out(i)->isa_Bool(); + if (!boolOut || !(boolOut->_test._test == BoolTest::eq || + boolOut->_test._test == BoolTest::ne)) { + return false; + } + } + return true; +} + //---------------------is_associative----------------------------- // Return TRUE if "n" is an associative binary node. If "base" is // not null, "n" must be re-associative with it. @@ -271,10 +290,10 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { if (base != nullptr) { assert(is_associative(base), "Base node should be associative"); int base_op = base->Opcode(); - if (base_op == Op_AddI || base_op == Op_SubI) { + if (base_op == Op_AddI || base_op == Op_SubI || base_op == Op_CmpI) { return op == Op_AddI || op == Op_SubI; } - if (base_op == Op_AddL || base_op == Op_SubL) { + if (base_op == Op_AddL || base_op == Op_SubL || base_op == Op_CmpL) { return op == Op_AddL || op == Op_SubL; } return op == base_op; @@ -285,7 +304,8 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { || op == Op_MulI || op == Op_MulL || op == Op_AndI || op == Op_AndL || op == Op_OrI || op == Op_OrL - || op == Op_XorI || op == Op_XorL; + || op == Op_XorI || op == Op_XorL + || is_associative_cmp(n); } } @@ -305,22 +325,29 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { // (inv2 - x) - inv1 => (-inv1 + inv2) - x // inv1 - (x + inv2) => ( inv1 - inv2) - x // -Node* IdealLoopTree::reassociate_add_sub(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop *phase) { - assert(n1->is_Add() || n1->is_Sub(), "Target node should be add or subtract"); +// Apply the same transormations to == and != +// inv1 == (x + inv2) => ( inv1 - inv2 ) == x +// inv1 == (x - inv2) => ( inv1 + inv2 ) == x +// inv1 == (inv2 - x) => (-inv1 + inv2 ) == x +// +Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop *phase) { + assert(n1->is_Add() || n1->is_Sub() || n1->is_Cmp(), "Target node should be add, subtract, or compare"); Node* n2 = n1->in(3 - inv1_idx); Node* inv1 = n1->in(inv1_idx); Node* inv2 = n2->in(inv2_idx); Node* x = n2->in(3 - inv2_idx); - bool neg_x = n2->is_Sub() && inv2_idx == 1; - bool neg_inv2 = n2->is_Sub() && inv2_idx == 2; - bool neg_inv1 = n1->is_Sub() && inv1_idx == 2; - if (n1->is_Sub() && inv1_idx == 1) { + bool neg_x = n2->is_Sub() && !n2->is_Cmp() && inv2_idx == 1; + bool neg_inv2 = (n2->is_Sub() && !n1->is_Cmp() && inv2_idx == 2) || + (n1->is_Cmp() && n2->is_Add()); + bool neg_inv1 = (n1->is_Sub() && !n1->is_Cmp() && inv1_idx == 2) || + (n1->is_Cmp() && inv2_idx == 1 && n2->is_Sub()); + if (n1->is_Sub() && !n1->is_Cmp() && inv1_idx == 1) { neg_x = !neg_x; neg_inv2 = !neg_inv2; } - bool is_int = n1->bottom_type()->isa_int() != nullptr; + bool is_int = n2->bottom_type()->isa_int() != nullptr; Node* inv1_c = phase->get_ctrl(inv1); Node* n_inv1; if (neg_inv1) { @@ -346,6 +373,9 @@ Node* IdealLoopTree::reassociate_add_sub(Node* n1, int inv1_idx, int inv2_idx, P inv = new AddINode(n_inv1, inv2); } phase->register_new_node(inv, phase->get_early_ctrl(inv)); + if (n1->is_Cmp()) { + return new CmpINode(x, inv); + } if (neg_x) { return new SubINode(inv, x); } else { @@ -358,6 +388,9 @@ Node* IdealLoopTree::reassociate_add_sub(Node* n1, int inv1_idx, int inv2_idx, P inv = new AddLNode(n_inv1, inv2); } phase->register_new_node(inv, phase->get_early_ctrl(inv)); + if (n1->is_Cmp()) { + return new CmpLNode(x, inv); + } if (neg_x) { return new SubLNode(inv, x); } else { @@ -396,7 +429,9 @@ Node* IdealLoopTree::reassociate(Node* n1, PhaseIdealLoop *phase) { case Op_AddL: case Op_SubI: case Op_SubL: - result = reassociate_add_sub(n1, inv1_idx, inv2_idx, phase); + case Op_CmpI: + case Op_CmpL: + result = reassociate_add_sub_cmp(n1, inv1_idx, inv2_idx, phase); break; case Op_MulI: case Op_MulL: diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 4a34d7e49baca..a646797111dd5 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -743,12 +743,14 @@ class IdealLoopTree : public ResourceObj { // Reassociate invariant binary expressions. Node* reassociate(Node* n1, PhaseIdealLoop *phase); // Reassociate invariant add and subtract expressions. - Node* reassociate_add_sub(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop *phase); + Node* reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop *phase); // Return nonzero index of invariant operand if invariant and variant // are combined with an associative binary. Helper for reassociate_invariants. int find_invariant(Node* n, PhaseIdealLoop *phase); // Return TRUE if "n" is associative. bool is_associative(Node* n, Node* base=nullptr); + // Return TRUE if "n" is an associative cmp node. + bool is_associative_cmp(Node* n); // Return true if n is invariant bool is_invariant(Node* n) const; diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java new file mode 100644 index 0000000000000..c26fac17a6198 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -0,0 +1,221 @@ +/* + * Copyright Amazon.com Inc. 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 + * 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. + */ + +package compiler.c2.loopopts; + +import compiler.lib.ir_framework.*; + +/* + * @test + * @bug 8250808 + * @summary Test loop invariant code motion through reassociation + * @library /test/lib / + * @run driver compiler.c2.loopopts.InvariantCodeMotionReassociateCmp + */ +public class InvariantCodeMotionReassociateCmp { + + public static void main(String[] args) { + TestFramework.run(); + } + + @DontInline + private void blackhole() {} + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.SUB_I, "1"}) + public void equalsAddInt(int inv1, int inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv2 - inv1 == i` + if (inv1 + i == inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.SUB_L, "1"}) + public void equalsAdtLong(long inv1, long inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv2 - inv1 == i` + if (inv1 + i == inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.SUB_I, "1"}) + public void equalsInvariantSubVariantInt(int inv1, int inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv1 - inv2 == i` + if (inv1 - i == inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.SUB_L, "1"}) + public void equalsInvariantSubVariantLong(long inv1, long inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv1 - inv2 == i` + if (inv1 - i == inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.ADD_I, "2"}) + public void equalsVariantSubInvariantInt(int inv1, int inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv1 + inv2 == i` + if (i - inv1 == inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.ADD_L, "1"}) + public void equalsVariantSubInvariantLong(long inv1, long inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv1 + inv2 == i` + if (i - inv1 == inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.SUB_I, "1"}) + public void notEqualsAddInt(int inv1, int inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv1 - inv2 != i` + if (inv1 + i != inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.SUB_L, "1"}) + public void notEqualsAdtLong(long inv1, long inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv1 - inv2 != i` + if (inv1 + i != inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.SUB_I, "1"}) + public void notEqualsInvariantSubVariantInt(int inv1, int inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv2 - inv1 != i` + if (inv1 - i != inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.SUB_L, "1"}) + public void notEqualsInvariantSubVariantLong(long inv1, long inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv2 - inv1 != i` + if (inv1 - i != inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.ADD_I, "2"}) + public void notEqualsVariantSubInvariantInt(int inv1, int inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv1 + inv2 != i` + if (i - inv1 != inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(counts = {IRNode.ADD_L, "1"}) + public void notEqualsVariantSubInvariantLong(long inv1, long inv2) { + for (int i = 0; i < 500; ++i) { + // Reassociate to `inv1 + inv2 != i` + if (i - inv1 != inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(failOn = {IRNode.SUB_I}) + public void leDontReassociate(int inv1, int inv2) { + for (int i = 0; i < 500; ++i) { + if (inv1 + i <= inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(failOn = {IRNode.SUB_I}) + public void gtDontReassociate(int inv1, int inv2) { + for (int i = 0; i < 500; ++i) { + if (inv1 + i > inv2) { + blackhole(); + } + } + } + + @Test + @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @IR(failOn = {IRNode.SUB_I}) + public void geDontReassociate(int inv1, int inv2) { + for (int i = 0; i < 500; ++i) { + if (inv1 + i >= inv2) { + blackhole(); + } + } + } +} + From dda874eb89383117ddda7a0eeead427fa9c5804d Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Wed, 17 Jan 2024 20:36:58 +0100 Subject: [PATCH 02/16] Formatting and comments --- src/hotspot/share/opto/loopTransform.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 771220472a567..0684c0a4a7e45 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -255,7 +255,7 @@ void IdealLoopTree::compute_profile_trip_cnt(PhaseIdealLoop *phase) { // Return nonzero index of invariant operand for an associative // binary operation of (nonconstant) invariant and variant values. // Helper for reassociate_invariants. -int IdealLoopTree::find_invariant(Node* n, PhaseIdealLoop *phase) { +int IdealLoopTree::find_invariant(Node* n, PhaseIdealLoop* phase) { bool in1_invar = this->is_invariant(n->in(1)); bool in2_invar = this->is_invariant(n->in(2)); if (in1_invar && !in2_invar) return 1; @@ -273,9 +273,9 @@ bool IdealLoopTree::is_associative_cmp(Node* n) { return false; } for (DUIterator i = n->outs(); n->has_out(i); i++) { - BoolNode *boolOut = n->out(i)->isa_Bool(); - if (!boolOut || !(boolOut->_test._test == BoolTest::eq || - boolOut->_test._test == BoolTest::ne)) { + BoolNode* boolOut = n->out(i)->isa_Bool(); + if (boolOut == nullptr || !(boolOut->_test._test == BoolTest::eq || + boolOut->_test._test == BoolTest::ne)) { return false; } } @@ -325,7 +325,7 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { // (inv2 - x) - inv1 => (-inv1 + inv2) - x // inv1 - (x + inv2) => ( inv1 - inv2) - x // -// Apply the same transormations to == and != +// Apply the same transformations to == and != // inv1 == (x + inv2) => ( inv1 - inv2 ) == x // inv1 == (x - inv2) => ( inv1 + inv2 ) == x // inv1 == (inv2 - x) => (-inv1 + inv2 ) == x @@ -337,6 +337,9 @@ Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_id Node* inv2 = n2->in(inv2_idx); Node* x = n2->in(3 - inv2_idx); + // Follow the comments for the transformations at the top of the function to + // determine whether x, inv1, or inv2 should be negative. Explicit checks that + // Sub nodes are not Cmp nodes are required because Cmp nodes are Sub nodes. bool neg_x = n2->is_Sub() && !n2->is_Cmp() && inv2_idx == 1; bool neg_inv2 = (n2->is_Sub() && !n1->is_Cmp() && inv2_idx == 2) || (n1->is_Cmp() && n2->is_Add()); From cb6d24b4263da4cafc635178492cc9186f1522cb Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Sat, 20 Jan 2024 01:30:42 +0100 Subject: [PATCH 03/16] Assert for n2. Variables for n1/n2 opcode. More concise comments. Overflow/random tests --- src/hotspot/share/opto/loopTransform.cpp | 27 +++++++++-------- .../InvariantCodeMotionReassociateCmp.java | 30 ++++++++++++------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 0684c0a4a7e45..812cdde8aace4 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -331,21 +331,24 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { // inv1 == (inv2 - x) => (-inv1 + inv2 ) == x // Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop *phase) { - assert(n1->is_Add() || n1->is_Sub() || n1->is_Cmp(), "Target node should be add, subtract, or compare"); Node* n2 = n1->in(3 - inv1_idx); + bool n1_is_sub = n1->is_Sub() && !n1->is_Cmp(); + bool n1_is_cmp = n1->is_Cmp(); + bool n2_is_sub = n2->is_Sub(); + assert(n1->is_Add() || n1_is_sub || n1_is_cmp, "Target node should be add, subtract, or compare"); + assert(n2->is_Add() || n2_is_sub, "Child node should be add or subtract"); Node* inv1 = n1->in(inv1_idx); Node* inv2 = n2->in(inv2_idx); Node* x = n2->in(3 - inv2_idx); - // Follow the comments for the transformations at the top of the function to - // determine whether x, inv1, or inv2 should be negative. Explicit checks that - // Sub nodes are not Cmp nodes are required because Cmp nodes are Sub nodes. - bool neg_x = n2->is_Sub() && !n2->is_Cmp() && inv2_idx == 1; - bool neg_inv2 = (n2->is_Sub() && !n1->is_Cmp() && inv2_idx == 2) || - (n1->is_Cmp() && n2->is_Add()); - bool neg_inv1 = (n1->is_Sub() && !n1->is_Cmp() && inv1_idx == 2) || - (n1->is_Cmp() && inv2_idx == 1 && n2->is_Sub()); - if (n1->is_Sub() && !n1->is_Cmp() && inv1_idx == 1) { + // Determine whether x, inv1, or inv2 should be negative in the transformed + // expression + bool neg_x = n2_is_sub && inv2_idx == 1; + bool neg_inv2 = + (n2_is_sub && !n1_is_cmp && inv2_idx == 2) || (n1_is_cmp && !n2_is_sub); + bool neg_inv1 = + (n1_is_sub && inv1_idx == 2) || (n1_is_cmp && inv2_idx == 1 && n2_is_sub); + if (n1_is_sub && inv1_idx == 1) { neg_x = !neg_x; neg_inv2 = !neg_inv2; } @@ -376,7 +379,7 @@ Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_id inv = new AddINode(n_inv1, inv2); } phase->register_new_node(inv, phase->get_early_ctrl(inv)); - if (n1->is_Cmp()) { + if (n1_is_cmp) { return new CmpINode(x, inv); } if (neg_x) { @@ -391,7 +394,7 @@ Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_id inv = new AddLNode(n_inv1, inv2); } phase->register_new_node(inv, phase->get_early_ctrl(inv)); - if (n1->is_Cmp()) { + if (n1_is_cmp) { return new CmpLNode(x, inv); } if (neg_x) { diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index c26fac17a6198..d5850d0306ab0 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -24,6 +24,7 @@ package compiler.c2.loopopts; import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; /* * @test @@ -42,7 +43,7 @@ public static void main(String[] args) { private void blackhole() {} @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.SUB_I, "1"}) public void equalsAddInt(int inv1, int inv2) { for (int i = 0; i < 500; ++i) { @@ -54,7 +55,7 @@ public void equalsAddInt(int inv1, int inv2) { } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.SUB_L, "1"}) public void equalsAdtLong(long inv1, long inv2) { for (int i = 0; i < 500; ++i) { @@ -114,7 +115,7 @@ public void equalsVariantSubInvariantLong(long inv1, long inv2) { } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.SUB_I, "1"}) public void notEqualsAddInt(int inv1, int inv2) { for (int i = 0; i < 500; ++i) { @@ -126,7 +127,7 @@ public void notEqualsAddInt(int inv1, int inv2) { } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.SUB_L, "1"}) public void notEqualsAdtLong(long inv1, long inv2) { for (int i = 0; i < 500; ++i) { @@ -162,7 +163,7 @@ public void notEqualsInvariantSubVariantLong(long inv1, long inv2) { } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.ADD_I, "2"}) public void notEqualsVariantSubInvariantInt(int inv1, int inv2) { for (int i = 0; i < 500; ++i) { @@ -174,7 +175,7 @@ public void notEqualsVariantSubInvariantInt(int inv1, int inv2) { } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.ADD_L, "1"}) public void notEqualsVariantSubInvariantLong(long inv1, long inv2) { for (int i = 0; i < 500; ++i) { @@ -189,7 +190,8 @@ public void notEqualsVariantSubInvariantLong(long inv1, long inv2) { @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) @IR(failOn = {IRNode.SUB_I}) public void leDontReassociate(int inv1, int inv2) { - for (int i = 0; i < 500; ++i) { + int i = 0; + for (; i < 500; ++i) { if (inv1 + i <= inv2) { blackhole(); } @@ -197,25 +199,31 @@ public void leDontReassociate(int inv1, int inv2) { } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.NUMBER_42, Argument.MIN}) @IR(failOn = {IRNode.SUB_I}) public void gtDontReassociate(int inv1, int inv2) { - for (int i = 0; i < 500; ++i) { + int i = 0; + for (; i < 500; ++i) { if (inv1 + i > inv2) { blackhole(); + break; } } + Asserts.assertEQ(i, 0, "illegal reassociation of a + b > c"); } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.NUMBER_42, Argument.MIN}) @IR(failOn = {IRNode.SUB_I}) public void geDontReassociate(int inv1, int inv2) { - for (int i = 0; i < 500; ++i) { + int i = 0; + for (; i < 500; ++i) { if (inv1 + i >= inv2) { blackhole(); + break; } } + Asserts.assertEQ(i, 0, "illegal reassociation of a + b >= c"); } } From 5ea7a53ae666472f1bf24f86e4cd9f169e10ff7b Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Mon, 22 Jan 2024 22:55:57 +0100 Subject: [PATCH 04/16] Formatting and fix typo --- src/hotspot/share/opto/loopTransform.cpp | 2 +- .../compiler/loopopts/InvariantCodeMotionReassociateCmp.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 812cdde8aace4..a13f0b98516bd 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -330,7 +330,7 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { // inv1 == (x - inv2) => ( inv1 + inv2 ) == x // inv1 == (inv2 - x) => (-inv1 + inv2 ) == x // -Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop *phase) { +Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop* phase) { Node* n2 = n1->in(3 - inv1_idx); bool n1_is_sub = n1->is_Sub() && !n1->is_Cmp(); bool n1_is_cmp = n1->is_Cmp(); diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index d5850d0306ab0..3ec76bf2176f1 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -57,7 +57,7 @@ public void equalsAddInt(int inv1, int inv2) { @Test @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.SUB_L, "1"}) - public void equalsAdtLong(long inv1, long inv2) { + public void equalsAddLong(long inv1, long inv2) { for (int i = 0; i < 500; ++i) { // Reassociate to `inv2 - inv1 == i` if (inv1 + i == inv2) { @@ -129,7 +129,7 @@ public void notEqualsAddInt(int inv1, int inv2) { @Test @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.SUB_L, "1"}) - public void notEqualsAdtLong(long inv1, long inv2) { + public void notEqualsAddLong(long inv1, long inv2) { for (int i = 0; i < 500; ++i) { // Reassociate to `inv1 - inv2 != i` if (inv1 + i != inv2) { From a08df7f7783a4916bbbbb939cde028820fa74afe Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Wed, 24 Jan 2024 23:47:56 +0100 Subject: [PATCH 05/16] reassociate_add_sub -> reassociate_add_sub_cmp --- src/hotspot/share/opto/loopTransform.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index a13f0b98516bd..5f2345218af74 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -309,7 +309,7 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { } } -//---------------------reassociate_add_sub------------------------ +//-------------------reassociate_add_sub_cmp--------------------- // Reassociate invariant add and subtract expressions: // // inv1 + (x + inv2) => ( inv1 + inv2) + x @@ -407,8 +407,8 @@ Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_id //---------------------reassociate----------------------------- // Reassociate invariant binary expressions with add/sub/mul/ -// and/or/xor operators. -// For add/sub expressions: see "reassociate_add_sub" +// and/or/xor/cmp operators. +// For add/sub/cmp expressions: see "reassociate_add_sub_cmp" // // For mul/and/or/xor expressions: // From 1603864fec589ba3e2e9e80a64db37dc94bb1f8d Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Sat, 27 Jan 2024 01:06:10 +0100 Subject: [PATCH 06/16] Small fixes and add check methods for tests --- src/hotspot/share/opto/loopTransform.cpp | 2 +- src/hotspot/share/opto/loopnode.hpp | 2 +- .../InvariantCodeMotionReassociateCmp.java | 38 +++++++++++++++---- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 5f2345218af74..ef8f78541c5e9 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -336,7 +336,7 @@ Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_id bool n1_is_cmp = n1->is_Cmp(); bool n2_is_sub = n2->is_Sub(); assert(n1->is_Add() || n1_is_sub || n1_is_cmp, "Target node should be add, subtract, or compare"); - assert(n2->is_Add() || n2_is_sub, "Child node should be add or subtract"); + assert(n2->is_Add() || (n2_is_sub && !n2->is_Cmp()), "Child node should be add or subtract"); Node* inv1 = n1->in(inv1_idx); Node* inv2 = n2->in(inv2_idx); Node* x = n2->in(3 - inv2_idx); diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index a646797111dd5..d6f348e55eb08 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -742,7 +742,7 @@ class IdealLoopTree : public ResourceObj { void reassociate_invariants(PhaseIdealLoop *phase); // Reassociate invariant binary expressions. Node* reassociate(Node* n1, PhaseIdealLoop *phase); - // Reassociate invariant add and subtract expressions. + // Reassociate invariant add, subtract, and compare expressions. Node* reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop *phase); // Return nonzero index of invariant operand if invariant and variant // are combined with an associative binary. Helper for reassociate_invariants. diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index 3ec76bf2176f1..c0a449b2722e0 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -28,8 +28,8 @@ /* * @test - * @bug 8250808 - * @summary Test loop invariant code motion through reassociation + * @bug 8323220 + * @summary Test loop invariant code motion for cmp nodes through reassociation * @library /test/lib / * @run driver compiler.c2.loopopts.InvariantCodeMotionReassociateCmp */ @@ -189,19 +189,28 @@ public void notEqualsVariantSubInvariantLong(long inv1, long inv2) { @Test @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) @IR(failOn = {IRNode.SUB_I}) - public void leDontReassociate(int inv1, int inv2) { + public int leDontReassociate(int inv1, int inv2) { int i = 0; for (; i < 500; ++i) { if (inv1 + i <= inv2) { blackhole(); + break; } } + return i; + } + + @Check(test = "leDontReassociate") + public void checkLeDontReassociate(int returnValue, TestInfo info) { + if (returnValue != 0) { + throw new RuntimeException("Illegal reassociation"); + } } @Test @Arguments({Argument.NUMBER_42, Argument.MIN}) @IR(failOn = {IRNode.SUB_I}) - public void gtDontReassociate(int inv1, int inv2) { + public int gtDontReassociate(int inv1, int inv2) { int i = 0; for (; i < 500; ++i) { if (inv1 + i > inv2) { @@ -209,13 +218,20 @@ public void gtDontReassociate(int inv1, int inv2) { break; } } - Asserts.assertEQ(i, 0, "illegal reassociation of a + b > c"); + return i; + } + + @Check(test = "gtDontReassociate") + public void checkGtDontReassociate(int returnValue, TestInfo info) { + if (returnValue != 0) { + throw new RuntimeException("Illegal reassociation"); + } } @Test @Arguments({Argument.NUMBER_42, Argument.MIN}) @IR(failOn = {IRNode.SUB_I}) - public void geDontReassociate(int inv1, int inv2) { + public int geDontReassociate(int inv1, int inv2) { int i = 0; for (; i < 500; ++i) { if (inv1 + i >= inv2) { @@ -223,7 +239,15 @@ public void geDontReassociate(int inv1, int inv2) { break; } } - Asserts.assertEQ(i, 0, "illegal reassociation of a + b >= c"); + return i; } + + @Check(test = "geDontReassociate") + public void checkGeDontReassociate(int returnValue, TestInfo info) { + if (returnValue != 0) { + throw new RuntimeException("Illegal reassociation"); + } + } + } From d1cd230c40de9cbcf3f97f4ec718a32b8eb4c08e Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Thu, 1 Feb 2024 23:06:47 +0100 Subject: [PATCH 07/16] Remove unused TestInfo parameter. Have some tests exit mid-loop. --- .../InvariantCodeMotionReassociateCmp.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index c0a449b2722e0..25075450dd956 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -201,14 +201,14 @@ public int leDontReassociate(int inv1, int inv2) { } @Check(test = "leDontReassociate") - public void checkLeDontReassociate(int returnValue, TestInfo info) { + public void checkLeDontReassociate(int returnValue) { if (returnValue != 0) { throw new RuntimeException("Illegal reassociation"); } } @Test - @Arguments({Argument.NUMBER_42, Argument.MIN}) + @Arguments({Argument.DEFAULT, Argument.NUMBER_42}) @IR(failOn = {IRNode.SUB_I}) public int gtDontReassociate(int inv1, int inv2) { int i = 0; @@ -222,14 +222,14 @@ public int gtDontReassociate(int inv1, int inv2) { } @Check(test = "gtDontReassociate") - public void checkGtDontReassociate(int returnValue, TestInfo info) { - if (returnValue != 0) { + public void checkGtDontReassociate(int returnValue) { + if (returnValue != 43) { throw new RuntimeException("Illegal reassociation"); } } @Test - @Arguments({Argument.NUMBER_42, Argument.MIN}) + @Arguments({Argument.DEFAULT, Argument.NUMBER_42}) @IR(failOn = {IRNode.SUB_I}) public int geDontReassociate(int inv1, int inv2) { int i = 0; @@ -243,11 +243,10 @@ public int geDontReassociate(int inv1, int inv2) { } @Check(test = "geDontReassociate") - public void checkGeDontReassociate(int returnValue, TestInfo info) { - if (returnValue != 0) { + public void checkGeDontReassociate(int returnValue) { + if (returnValue != 42) { throw new RuntimeException("Illegal reassociation"); } } - } From f3ff5471ee3ec9f83bd117b5d734ae0e8f19e978 Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Thu, 8 Feb 2024 08:42:38 +0100 Subject: [PATCH 08/16] Add some correctness tests where we do reassociate --- .../InvariantCodeMotionReassociateCmp.java | 60 +++++++++++++++---- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index 25075450dd956..c2cb02ffedf79 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -67,27 +67,47 @@ public void equalsAddLong(long inv1, long inv2) { } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.NUMBER_42, Argument.DEFAULT}) @IR(counts = {IRNode.SUB_I, "1"}) - public void equalsInvariantSubVariantInt(int inv1, int inv2) { - for (int i = 0; i < 500; ++i) { + public int equalsInvariantSubVariantInt(int inv1, int inv2) { + int i = 0; + for (; i < 500; ++i) { // Reassociate to `inv1 - inv2 == i` if (inv1 - i == inv2) { blackhole(); + break; } } + return i; + } + + @Check(test = "equalsInvariantSubVariantInt") + public void checkEqualsInvariantSubVariantInt(int returnValue) { + if (returnValue != 42) { + throw new RuntimeException("Illegal reassociation"); + } } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments({Argument.NUMBER_42, Argument.DEFAULT}) @IR(counts = {IRNode.SUB_L, "1"}) - public void equalsInvariantSubVariantLong(long inv1, long inv2) { - for (int i = 0; i < 500; ++i) { + public int equalsInvariantSubVariantLong(long inv1, long inv2) { + int i = 0; + for (; i < 500; ++i) { // Reassociate to `inv1 - inv2 == i` if (inv1 - i == inv2) { blackhole(); + break; } } + return i; + } + + @Check(test = "equalsInvariantSubVariantLong") + public void checkEqualsInvariantSubVariantLong(int returnValue) { + if (returnValue != 42) { + throw new RuntimeException("Illegal reassociation"); + } } @Test @@ -141,25 +161,45 @@ public void notEqualsAddLong(long inv1, long inv2) { @Test @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) @IR(counts = {IRNode.SUB_I, "1"}) - public void notEqualsInvariantSubVariantInt(int inv1, int inv2) { - for (int i = 0; i < 500; ++i) { + public int notEqualsInvariantSubVariantInt(int inv1, int inv2) { + int i = 0; + for (; i < 500; ++i) { // Reassociate to `inv2 - inv1 != i` if (inv1 - i != inv2) { blackhole(); + break; } } + return i; + } + + @Check(test = "notEqualsInvariantSubVariantInt") + public void checkNotEqualsInvariantSubVariantInt(int returnValue) { + if (returnValue != 1) { + throw new RuntimeException("Illegal reassociation"); + } } @Test @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) @IR(counts = {IRNode.SUB_L, "1"}) - public void notEqualsInvariantSubVariantLong(long inv1, long inv2) { - for (int i = 0; i < 500; ++i) { + public int notEqualsInvariantSubVariantLong(long inv1, long inv2) { + int i = 0; + for (; i < 500; ++i) { // Reassociate to `inv2 - inv1 != i` if (inv1 - i != inv2) { blackhole(); + break; } } + return i; + } + + @Check(test = "notEqualsInvariantSubVariantLong") + public void checkNotEqualsInvariantSubVariantLong(int returnValue) { + if (returnValue != 1) { + throw new RuntimeException("Illegal reassociation"); + } } @Test From b29c557cf8727e61d7406cbaea16a73b5c4b7ae0 Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Thu, 8 Feb 2024 22:34:32 +0100 Subject: [PATCH 09/16] Add correctness test for some random tests with random inputs --- .../InvariantCodeMotionReassociateCmp.java | 53 +++++++++++++------ 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index c2cb02ffedf79..df32a671f616d 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -34,6 +34,7 @@ * @run driver compiler.c2.loopopts.InvariantCodeMotionReassociateCmp */ public class InvariantCodeMotionReassociateCmp { + private static final int size = 500; public static void main(String[] args) { TestFramework.run(); @@ -45,25 +46,47 @@ private void blackhole() {} @Test @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.SUB_I, "1"}) - public void equalsAddInt(int inv1, int inv2) { - for (int i = 0; i < 500; ++i) { + public int[] equalsAddInt(int inv1, int inv2) { + int i = 0; + for (; i < size; ++i) { // Reassociate to `inv2 - inv1 == i` if (inv1 + i == inv2) { blackhole(); + break; } } + return new int[]{i, inv1, inv2}; + } + + @Check(test = "equalsAddInt") + public void checkEqualsAddInt(int[] returnValue) { + if (returnValue[0] != size && returnValue[0] + returnValue[1] != returnValue[2]) { + throw new RuntimeException("Illegal reassociation: i=" + returnValue[0] + ", inv1=" + returnValue[1] + + ", inv2=" + returnValue[2]); + } } @Test @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.SUB_L, "1"}) - public void equalsAddLong(long inv1, long inv2) { - for (int i = 0; i < 500; ++i) { + public long[] equalsAddLong(long inv1, long inv2) { + int i = 0; + for (; i < size; ++i) { // Reassociate to `inv2 - inv1 == i` if (inv1 + i == inv2) { blackhole(); + break; } } + return new long[]{i, inv1, inv2}; + } + + @Check(test = "equalsAddLong") + public void checkEqualsAddLong(long[] returnValue) { + if (returnValue[0] != size && returnValue[0] + returnValue[1] != returnValue[2]) { + throw new RuntimeException("Illegal reassociation: i=" + returnValue[0] + ", inv1=" + returnValue[1] + + ", inv2=" + returnValue[2]); + } } @Test @@ -71,7 +94,7 @@ public void equalsAddLong(long inv1, long inv2) { @IR(counts = {IRNode.SUB_I, "1"}) public int equalsInvariantSubVariantInt(int inv1, int inv2) { int i = 0; - for (; i < 500; ++i) { + for (; i < size; ++i) { // Reassociate to `inv1 - inv2 == i` if (inv1 - i == inv2) { blackhole(); @@ -93,7 +116,7 @@ public void checkEqualsInvariantSubVariantInt(int returnValue) { @IR(counts = {IRNode.SUB_L, "1"}) public int equalsInvariantSubVariantLong(long inv1, long inv2) { int i = 0; - for (; i < 500; ++i) { + for (; i < size; ++i) { // Reassociate to `inv1 - inv2 == i` if (inv1 - i == inv2) { blackhole(); @@ -114,7 +137,7 @@ public void checkEqualsInvariantSubVariantLong(int returnValue) { @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) @IR(counts = {IRNode.ADD_I, "2"}) public void equalsVariantSubInvariantInt(int inv1, int inv2) { - for (int i = 0; i < 500; ++i) { + for (int i = 0; i < size; ++i) { // Reassociate to `inv1 + inv2 == i` if (i - inv1 == inv2) { blackhole(); @@ -126,7 +149,7 @@ public void equalsVariantSubInvariantInt(int inv1, int inv2) { @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) @IR(counts = {IRNode.ADD_L, "1"}) public void equalsVariantSubInvariantLong(long inv1, long inv2) { - for (int i = 0; i < 500; ++i) { + for (int i = 0; i < size; ++i) { // Reassociate to `inv1 + inv2 == i` if (i - inv1 == inv2) { blackhole(); @@ -150,7 +173,7 @@ public void notEqualsAddInt(int inv1, int inv2) { @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.SUB_L, "1"}) public void notEqualsAddLong(long inv1, long inv2) { - for (int i = 0; i < 500; ++i) { + for (int i = 0; i < size; ++i) { // Reassociate to `inv1 - inv2 != i` if (inv1 + i != inv2) { blackhole(); @@ -163,7 +186,7 @@ public void notEqualsAddLong(long inv1, long inv2) { @IR(counts = {IRNode.SUB_I, "1"}) public int notEqualsInvariantSubVariantInt(int inv1, int inv2) { int i = 0; - for (; i < 500; ++i) { + for (; i < size; ++i) { // Reassociate to `inv2 - inv1 != i` if (inv1 - i != inv2) { blackhole(); @@ -185,7 +208,7 @@ public void checkNotEqualsInvariantSubVariantInt(int returnValue) { @IR(counts = {IRNode.SUB_L, "1"}) public int notEqualsInvariantSubVariantLong(long inv1, long inv2) { int i = 0; - for (; i < 500; ++i) { + for (; i < size; ++i) { // Reassociate to `inv2 - inv1 != i` if (inv1 - i != inv2) { blackhole(); @@ -218,7 +241,7 @@ public void notEqualsVariantSubInvariantInt(int inv1, int inv2) { @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) @IR(counts = {IRNode.ADD_L, "1"}) public void notEqualsVariantSubInvariantLong(long inv1, long inv2) { - for (int i = 0; i < 500; ++i) { + for (int i = 0; i < size; ++i) { // Reassociate to `inv1 + inv2 != i` if (i - inv1 != inv2) { blackhole(); @@ -231,7 +254,7 @@ public void notEqualsVariantSubInvariantLong(long inv1, long inv2) { @IR(failOn = {IRNode.SUB_I}) public int leDontReassociate(int inv1, int inv2) { int i = 0; - for (; i < 500; ++i) { + for (; i < size; ++i) { if (inv1 + i <= inv2) { blackhole(); break; @@ -252,7 +275,7 @@ public void checkLeDontReassociate(int returnValue) { @IR(failOn = {IRNode.SUB_I}) public int gtDontReassociate(int inv1, int inv2) { int i = 0; - for (; i < 500; ++i) { + for (; i < size; ++i) { if (inv1 + i > inv2) { blackhole(); break; @@ -273,7 +296,7 @@ public void checkGtDontReassociate(int returnValue) { @IR(failOn = {IRNode.SUB_I}) public int geDontReassociate(int inv1, int inv2) { int i = 0; - for (; i < 500; ++i) { + for (; i < size; ++i) { if (inv1 + i >= inv2) { blackhole(); break; From ad073ef26da2db2393a7692529a3b84f50a6f21a Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Fri, 16 Feb 2024 01:44:55 +0100 Subject: [PATCH 10/16] Update test to utilize @setup method for arguments --- .../InvariantCodeMotionReassociateCmp.java | 245 +++++++++++++----- 1 file changed, 176 insertions(+), 69 deletions(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index df32a671f616d..843876fd56884 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -25,6 +25,8 @@ import compiler.lib.ir_framework.*; import jdk.test.lib.Asserts; +import jdk.test.lib.Utils; +import java.util.Random; /* * @test @@ -34,7 +36,10 @@ * @run driver compiler.c2.loopopts.InvariantCodeMotionReassociateCmp */ public class InvariantCodeMotionReassociateCmp { + private static final Random RANDOM = Utils.getRandomInstance(); private static final int size = 500; + private int inv1; + private int inv2; public static void main(String[] args) { TestFramework.run(); @@ -43,10 +48,52 @@ public static void main(String[] args) { @DontInline private void blackhole() {} + @Setup + public Object[] setupEq() { + inv1 = RANDOM.nextInt(); + inv2 = inv1 + RANDOM.nextInt(size * 2); + return new Object[] { inv1, inv2 }; + } + + @Setup + public Object[] setupNe() { + inv1 = RANDOM.nextInt(); + if (RANDOM.nextInt() % 7 == 0) { + if (inv1 == 0) { + inv1 = 1; + } + // Setup inputs to be equals sometimes to avoid uncommon traps + inv2 = inv1; + } else { + inv2 = inv1 + RANDOM.nextInt(size * 2) + 1; + } + return new Object[] { inv1, inv2 }; + } + public void fail(int returnValue) { + throw new RuntimeException("Illegal reassociation: i=" + returnValue + ", inv1=" + inv1 + + ", inv2=" + inv2); + } + + public void checkEq(int returnValue) { + int invDiff = inv2 - inv1; + if ((invDiff < size && returnValue != invDiff) || (invDiff + >= size && returnValue != size)) { + fail(returnValue); + } + } + + public void checkNe(int returnValue) { + int invDiff = inv2 - inv1; + if ((invDiff != 0 && returnValue != 0) || (invDiff + == 0 && returnValue != 1)) { + fail(returnValue); + } + } + @Test - @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @Arguments(setup = "setupEq") @IR(counts = {IRNode.SUB_I, "1"}) - public int[] equalsAddInt(int inv1, int inv2) { + public int equalsAddInt(int inv1, int inv2) { int i = 0; for (; i < size; ++i) { // Reassociate to `inv2 - inv1 == i` @@ -55,21 +102,18 @@ public int[] equalsAddInt(int inv1, int inv2) { break; } } - return new int[]{i, inv1, inv2}; + return i; } @Check(test = "equalsAddInt") - public void checkEqualsAddInt(int[] returnValue) { - if (returnValue[0] != size && returnValue[0] + returnValue[1] != returnValue[2]) { - throw new RuntimeException("Illegal reassociation: i=" + returnValue[0] + ", inv1=" + returnValue[1] - + ", inv2=" + returnValue[2]); - } + public void checkEqualsAddInt(int returnValue) { + checkEq(returnValue); } @Test - @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @Arguments(setup = "setupEq") @IR(counts = {IRNode.SUB_L, "1"}) - public long[] equalsAddLong(long inv1, long inv2) { + public int equalsAddLong(long inv1, long inv2) { int i = 0; for (; i < size; ++i) { // Reassociate to `inv2 - inv1 == i` @@ -78,25 +122,22 @@ public long[] equalsAddLong(long inv1, long inv2) { break; } } - return new long[]{i, inv1, inv2}; + return i; } @Check(test = "equalsAddLong") - public void checkEqualsAddLong(long[] returnValue) { - if (returnValue[0] != size && returnValue[0] + returnValue[1] != returnValue[2]) { - throw new RuntimeException("Illegal reassociation: i=" + returnValue[0] + ", inv1=" + returnValue[1] - + ", inv2=" + returnValue[2]); - } + public void checkEqualsAddLong(int returnValue) { + checkEq(returnValue); } @Test - @Arguments({Argument.NUMBER_42, Argument.DEFAULT}) + @Arguments(setup = "setupEq") @IR(counts = {IRNode.SUB_I, "1"}) public int equalsInvariantSubVariantInt(int inv1, int inv2) { int i = 0; for (; i < size; ++i) { // Reassociate to `inv1 - inv2 == i` - if (inv1 - i == inv2) { + if (inv2 - i == inv1) { blackhole(); break; } @@ -106,19 +147,17 @@ public int equalsInvariantSubVariantInt(int inv1, int inv2) { @Check(test = "equalsInvariantSubVariantInt") public void checkEqualsInvariantSubVariantInt(int returnValue) { - if (returnValue != 42) { - throw new RuntimeException("Illegal reassociation"); - } + checkEq(returnValue); } @Test - @Arguments({Argument.NUMBER_42, Argument.DEFAULT}) + @Arguments(setup = "setupEq") @IR(counts = {IRNode.SUB_L, "1"}) public int equalsInvariantSubVariantLong(long inv1, long inv2) { int i = 0; for (; i < size; ++i) { // Reassociate to `inv1 - inv2 == i` - if (inv1 - i == inv2) { + if (inv2 - i == inv1) { blackhole(); break; } @@ -128,61 +167,92 @@ public int equalsInvariantSubVariantLong(long inv1, long inv2) { @Check(test = "equalsInvariantSubVariantLong") public void checkEqualsInvariantSubVariantLong(int returnValue) { - if (returnValue != 42) { - throw new RuntimeException("Illegal reassociation"); - } + checkEq(returnValue); } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) - @IR(counts = {IRNode.ADD_I, "2"}) - public void equalsVariantSubInvariantInt(int inv1, int inv2) { - for (int i = 0; i < size; ++i) { + @Arguments(setup = "setupEq") + @IR(counts = {IRNode.SUB_I, "1"}) + public int equalsVariantSubInvariantInt(int inv1, int inv2) { + int i = 0; + for (; i < size; ++i) { // Reassociate to `inv1 + inv2 == i` - if (i - inv1 == inv2) { + if (i - inv2 == -inv1) { blackhole(); + break; } } + return i; + } + + @Check(test = "equalsVariantSubInvariantInt") + public void checkEqualsVariantSubInvariantInt(int returnValue) { + checkEq(returnValue); } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) - @IR(counts = {IRNode.ADD_L, "1"}) - public void equalsVariantSubInvariantLong(long inv1, long inv2) { - for (int i = 0; i < size; ++i) { + @Arguments(setup = "setupEq") + @IR(counts = {IRNode.SUB_L, "1"}) + public int equalsVariantSubInvariantLong(long inv1, long inv2) { + int i = 0; + for (; i < size; ++i) { // Reassociate to `inv1 + inv2 == i` - if (i - inv1 == inv2) { + if (i - inv2 == -inv1) { blackhole(); + break; } } + return i; + } + + @Check(test = "equalsVariantSubInvariantLong") + public void checkEqualsVariantSubInvariantLong(int returnValue) { + checkEq(returnValue); } + @Test - @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @Arguments(setup = "setupNe") @IR(counts = {IRNode.SUB_I, "1"}) - public void notEqualsAddInt(int inv1, int inv2) { - for (int i = 0; i < 500; ++i) { + public int notEqualsAddInt(int inv1, int inv2) { + int i = 0; + for (; i < 500; ++i) { // Reassociate to `inv1 - inv2 != i` if (inv1 + i != inv2) { blackhole(); + break; } } + return i; + } + + @Check(test = "notEqualsAddInt") + public void checkNotEqualsAddInt(int returnValue) { + checkNe(returnValue); } @Test - @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @Arguments(setup = "setupNe") @IR(counts = {IRNode.SUB_L, "1"}) - public void notEqualsAddLong(long inv1, long inv2) { - for (int i = 0; i < size; ++i) { + public int notEqualsAddLong(long inv1, long inv2) { + int i = 0; + for (; i < size; ++i) { // Reassociate to `inv1 - inv2 != i` if (inv1 + i != inv2) { blackhole(); + break; } } + return i; + } + + @Check(test = "notEqualsAddLong") + public void checkNotEqualsAddLong(int returnValue) { + checkNe(returnValue); } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments(setup = "setupNe") @IR(counts = {IRNode.SUB_I, "1"}) public int notEqualsInvariantSubVariantInt(int inv1, int inv2) { int i = 0; @@ -198,13 +268,11 @@ public int notEqualsInvariantSubVariantInt(int inv1, int inv2) { @Check(test = "notEqualsInvariantSubVariantInt") public void checkNotEqualsInvariantSubVariantInt(int returnValue) { - if (returnValue != 1) { - throw new RuntimeException("Illegal reassociation"); - } + checkNe(returnValue); } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments(setup = "setupNe") @IR(counts = {IRNode.SUB_L, "1"}) public int notEqualsInvariantSubVariantLong(long inv1, long inv2) { int i = 0; @@ -220,37 +288,73 @@ public int notEqualsInvariantSubVariantLong(long inv1, long inv2) { @Check(test = "notEqualsInvariantSubVariantLong") public void checkNotEqualsInvariantSubVariantLong(int returnValue) { - if (returnValue != 1) { - throw new RuntimeException("Illegal reassociation"); - } + checkNe(returnValue); } @Test - @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) - @IR(counts = {IRNode.ADD_I, "2"}) - public void notEqualsVariantSubInvariantInt(int inv1, int inv2) { - for (int i = 0; i < 500; ++i) { + @Arguments(setup = "setupNe") + @IR(counts = {IRNode.SUB_I, "1"}) + public int notEqualsVariantSubInvariantInt(int inv1, int inv2) { + int i = 0; + for (; i < 500; ++i) { // Reassociate to `inv1 + inv2 != i` - if (i - inv1 != inv2) { + if (i - inv2 != -inv1) { blackhole(); + break; } } + return i; + } + + @Check(test = "notEqualsVariantSubInvariantInt") + public void checkNotEqualsVariantSubInvariantInt(int returnValue) { + checkNe(returnValue); } @Test - @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) - @IR(counts = {IRNode.ADD_L, "1"}) - public void notEqualsVariantSubInvariantLong(long inv1, long inv2) { - for (int i = 0; i < size; ++i) { + @Arguments(setup = "setupNe") + @IR(counts = {IRNode.SUB_L, "1"}) + public int notEqualsVariantSubInvariantLong(long inv1, long inv2) { + int i = 0; + for (; i < size; ++i) { // Reassociate to `inv1 + inv2 != i` - if (i - inv1 != inv2) { + if (i - inv2 != -inv1) { blackhole(); + break; + } + } + return i; + } + + @Check(test = "notEqualsVariantSubInvariantLong") + public void checkNotEqualsVariantSubInvariantLong(int returnValue) { + checkNe(returnValue); + } + + @Test + @Arguments(setup = "setupEq") + @IR(failOn = {IRNode.SUB_I}) + public int ltDontReassociate(int inv1, int inv2) { + int i = 0; + for (; i < size; ++i) { + if (inv1 + i < inv2) { + blackhole(); + break; } } + return i; + } + + @Check(test = "ltDontReassociate") + public void checkLtDontReassociate(int returnValue) { + int sum = inv1 + returnValue; + if ((returnValue < size && sum >= inv2) || (returnValue > size && sum < inv2)) { + fail(returnValue); + } } @Test - @Arguments({Argument.NUMBER_42, Argument.NUMBER_42}) + @Arguments(setup = "setupEq") @IR(failOn = {IRNode.SUB_I}) public int leDontReassociate(int inv1, int inv2) { int i = 0; @@ -265,13 +369,14 @@ public int leDontReassociate(int inv1, int inv2) { @Check(test = "leDontReassociate") public void checkLeDontReassociate(int returnValue) { - if (returnValue != 0) { - throw new RuntimeException("Illegal reassociation"); + int sum = inv1 + returnValue; + if ((returnValue < size && sum > inv2) || (returnValue > size && sum <= inv2)) { + fail(returnValue); } } @Test - @Arguments({Argument.DEFAULT, Argument.NUMBER_42}) + @Arguments(setup = "setupEq") @IR(failOn = {IRNode.SUB_I}) public int gtDontReassociate(int inv1, int inv2) { int i = 0; @@ -286,13 +391,14 @@ public int gtDontReassociate(int inv1, int inv2) { @Check(test = "gtDontReassociate") public void checkGtDontReassociate(int returnValue) { - if (returnValue != 43) { - throw new RuntimeException("Illegal reassociation"); + int sum = inv1 + returnValue; + if ((returnValue < size && sum <= inv2) || (returnValue > size && sum > inv2)) { + fail(returnValue); } } @Test - @Arguments({Argument.DEFAULT, Argument.NUMBER_42}) + @Arguments(setup = "setupEq") @IR(failOn = {IRNode.SUB_I}) public int geDontReassociate(int inv1, int inv2) { int i = 0; @@ -307,8 +413,9 @@ public int geDontReassociate(int inv1, int inv2) { @Check(test = "geDontReassociate") public void checkGeDontReassociate(int returnValue) { - if (returnValue != 42) { - throw new RuntimeException("Illegal reassociation"); + int sum = inv1 + returnValue; + if ((returnValue < size && sum < inv2) || (returnValue > size && sum >= inv2)) { + fail(returnValue); } } } From 3e573b08162fda413da1dd2997e6dcdd175969d1 Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Fri, 16 Feb 2024 10:11:34 +0100 Subject: [PATCH 11/16] Make inputs deterministic. Make size an arg. Fix comments. Formatting. --- .../InvariantCodeMotionReassociateCmp.java | 140 ++++++++---------- 1 file changed, 65 insertions(+), 75 deletions(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index 843876fd56884..05707820bcb93 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -37,7 +37,7 @@ */ public class InvariantCodeMotionReassociateCmp { private static final Random RANDOM = Utils.getRandomInstance(); - private static final int size = 500; + private int size; private int inv1; private int inv2; @@ -49,26 +49,19 @@ public static void main(String[] args) { private void blackhole() {} @Setup - public Object[] setupEq() { - inv1 = RANDOM.nextInt(); - inv2 = inv1 + RANDOM.nextInt(size * 2); - return new Object[] { inv1, inv2 }; - } - - @Setup - public Object[] setupNe() { - inv1 = RANDOM.nextInt(); + public Object[] setup(SetupInfo info) { + int count = info.invocationCounter(); + size = count + 500; + inv1 = count; if (RANDOM.nextInt() % 7 == 0) { - if (inv1 == 0) { - inv1 = 1; - } // Setup inputs to be equals sometimes to avoid uncommon traps inv2 = inv1; } else { - inv2 = inv1 + RANDOM.nextInt(size * 2) + 1; + inv2 = count * 2; } - return new Object[] { inv1, inv2 }; + return new Object[] { inv1, inv2, size }; } + public void fail(int returnValue) { throw new RuntimeException("Illegal reassociation: i=" + returnValue + ", inv1=" + inv1 + ", inv2=" + inv2); @@ -76,29 +69,27 @@ public void fail(int returnValue) { public void checkEq(int returnValue) { int invDiff = inv2 - inv1; - if ((invDiff < size && returnValue != invDiff) || (invDiff - >= size && returnValue != size)) { + if ((invDiff < size && returnValue != invDiff) || (invDiff >= size && returnValue != size)) { fail(returnValue); } } public void checkNe(int returnValue) { int invDiff = inv2 - inv1; - if ((invDiff != 0 && returnValue != 0) || (invDiff - == 0 && returnValue != 1)) { + if ((invDiff != 0 && returnValue != 0) || (invDiff == 0 && returnValue != 1)) { fail(returnValue); } } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_I, "1"}) - public int equalsAddInt(int inv1, int inv2) { + public int equalsAddInt(int inv1, int inv2, int size) { int i = 0; for (; i < size; ++i) { + blackhole(); // Reassociate to `inv2 - inv1 == i` if (inv1 + i == inv2) { - blackhole(); break; } } @@ -111,14 +102,14 @@ public void checkEqualsAddInt(int returnValue) { } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_L, "1"}) - public int equalsAddLong(long inv1, long inv2) { + public int equalsAddLong(long inv1, long inv2, int size) { int i = 0; for (; i < size; ++i) { + blackhole(); // Reassociate to `inv2 - inv1 == i` if (inv1 + i == inv2) { - blackhole(); break; } } @@ -131,14 +122,14 @@ public void checkEqualsAddLong(int returnValue) { } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_I, "1"}) - public int equalsInvariantSubVariantInt(int inv1, int inv2) { + public int equalsInvariantSubVariantInt(int inv1, int inv2, int size) { int i = 0; for (; i < size; ++i) { + blackhole(); // Reassociate to `inv1 - inv2 == i` if (inv2 - i == inv1) { - blackhole(); break; } } @@ -151,14 +142,14 @@ public void checkEqualsInvariantSubVariantInt(int returnValue) { } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_L, "1"}) - public int equalsInvariantSubVariantLong(long inv1, long inv2) { + public int equalsInvariantSubVariantLong(long inv1, long inv2, int size) { int i = 0; for (; i < size; ++i) { - // Reassociate to `inv1 - inv2 == i` + blackhole(); + // Reassociate to `inv2 - inv1 == i` if (inv2 - i == inv1) { - blackhole(); break; } } @@ -171,14 +162,14 @@ public void checkEqualsInvariantSubVariantLong(int returnValue) { } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_I, "1"}) - public int equalsVariantSubInvariantInt(int inv1, int inv2) { + public int equalsVariantSubInvariantInt(int inv1, int inv2, int size) { int i = 0; for (; i < size; ++i) { - // Reassociate to `inv1 + inv2 == i` + blackhole(); + // Reassociate to `inv2 - inv1 == i` if (i - inv2 == -inv1) { - blackhole(); break; } } @@ -191,14 +182,14 @@ public void checkEqualsVariantSubInvariantInt(int returnValue) { } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_L, "1"}) - public int equalsVariantSubInvariantLong(long inv1, long inv2) { + public int equalsVariantSubInvariantLong(long inv1, long inv2, int size) { int i = 0; for (; i < size; ++i) { - // Reassociate to `inv1 + inv2 == i` + blackhole(); + // Reassociate to `inv2 - inv1 == i` if (i - inv2 == -inv1) { - blackhole(); break; } } @@ -212,14 +203,14 @@ public void checkEqualsVariantSubInvariantLong(int returnValue) { @Test - @Arguments(setup = "setupNe") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_I, "1"}) - public int notEqualsAddInt(int inv1, int inv2) { + public int notEqualsAddInt(int inv1, int inv2, int size) { int i = 0; for (; i < 500; ++i) { + blackhole(); // Reassociate to `inv1 - inv2 != i` if (inv1 + i != inv2) { - blackhole(); break; } } @@ -232,14 +223,14 @@ public void checkNotEqualsAddInt(int returnValue) { } @Test - @Arguments(setup = "setupNe") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_L, "1"}) - public int notEqualsAddLong(long inv1, long inv2) { + public int notEqualsAddLong(long inv1, long inv2, int size) { int i = 0; for (; i < size; ++i) { + blackhole(); // Reassociate to `inv1 - inv2 != i` if (inv1 + i != inv2) { - blackhole(); break; } } @@ -252,14 +243,14 @@ public void checkNotEqualsAddLong(int returnValue) { } @Test - @Arguments(setup = "setupNe") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_I, "1"}) - public int notEqualsInvariantSubVariantInt(int inv1, int inv2) { + public int notEqualsInvariantSubVariantInt(int inv1, int inv2, int size) { int i = 0; for (; i < size; ++i) { - // Reassociate to `inv2 - inv1 != i` + blackhole(); + // Reassociate to `inv1 - inv2 != i` if (inv1 - i != inv2) { - blackhole(); break; } } @@ -272,14 +263,14 @@ public void checkNotEqualsInvariantSubVariantInt(int returnValue) { } @Test - @Arguments(setup = "setupNe") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_L, "1"}) - public int notEqualsInvariantSubVariantLong(long inv1, long inv2) { + public int notEqualsInvariantSubVariantLong(long inv1, long inv2, int size) { int i = 0; for (; i < size; ++i) { - // Reassociate to `inv2 - inv1 != i` + blackhole(); + // Reassociate to `inv1 - inv2 != i` if (inv1 - i != inv2) { - blackhole(); break; } } @@ -292,14 +283,14 @@ public void checkNotEqualsInvariantSubVariantLong(int returnValue) { } @Test - @Arguments(setup = "setupNe") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_I, "1"}) - public int notEqualsVariantSubInvariantInt(int inv1, int inv2) { + public int notEqualsVariantSubInvariantInt(int inv1, int inv2, int size) { int i = 0; for (; i < 500; ++i) { - // Reassociate to `inv1 + inv2 != i` + blackhole(); + // Reassociate to `inv2 - inv1 != i` if (i - inv2 != -inv1) { - blackhole(); break; } } @@ -312,14 +303,14 @@ public void checkNotEqualsVariantSubInvariantInt(int returnValue) { } @Test - @Arguments(setup = "setupNe") + @Arguments(setup = "setup") @IR(counts = {IRNode.SUB_L, "1"}) - public int notEqualsVariantSubInvariantLong(long inv1, long inv2) { + public int notEqualsVariantSubInvariantLong(long inv1, long inv2, int size) { int i = 0; for (; i < size; ++i) { - // Reassociate to `inv1 + inv2 != i` + blackhole(); + // Reassociate to `inv1 - inv1 != i` if (i - inv2 != -inv1) { - blackhole(); break; } } @@ -332,13 +323,13 @@ public void checkNotEqualsVariantSubInvariantLong(int returnValue) { } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(failOn = {IRNode.SUB_I}) - public int ltDontReassociate(int inv1, int inv2) { + public int ltDontReassociate(int inv1, int inv2, int size) { int i = 0; for (; i < size; ++i) { + blackhole(); if (inv1 + i < inv2) { - blackhole(); break; } } @@ -354,13 +345,13 @@ public void checkLtDontReassociate(int returnValue) { } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(failOn = {IRNode.SUB_I}) - public int leDontReassociate(int inv1, int inv2) { + public int leDontReassociate(int inv1, int inv2, int size) { int i = 0; for (; i < size; ++i) { + blackhole(); if (inv1 + i <= inv2) { - blackhole(); break; } } @@ -376,13 +367,13 @@ public void checkLeDontReassociate(int returnValue) { } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(failOn = {IRNode.SUB_I}) - public int gtDontReassociate(int inv1, int inv2) { + public int gtDontReassociate(int inv1, int inv2, int size) { int i = 0; for (; i < size; ++i) { + blackhole(); if (inv1 + i > inv2) { - blackhole(); break; } } @@ -398,13 +389,13 @@ public void checkGtDontReassociate(int returnValue) { } @Test - @Arguments(setup = "setupEq") + @Arguments(setup = "setup") @IR(failOn = {IRNode.SUB_I}) - public int geDontReassociate(int inv1, int inv2) { + public int geDontReassociate(int inv1, int inv2, int size) { int i = 0; for (; i < size; ++i) { + blackhole(); if (inv1 + i >= inv2) { - blackhole(); break; } } @@ -419,4 +410,3 @@ public void checkGeDontReassociate(int returnValue) { } } } - From b151293dcb8d15bddb60b33784513fc39a4290a9 Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Thu, 21 Mar 2024 06:30:57 +0100 Subject: [PATCH 12/16] Add tests for add/sub reassociation --- .../InvariantCodeMotionReassociateAddSub.java | 525 ++++++++++++++++++ .../InvariantCodeMotionReassociateCmp.java | 4 +- 2 files changed, 527 insertions(+), 2 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java new file mode 100644 index 0000000000000..4e8daf3d163c6 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java @@ -0,0 +1,525 @@ +/* + * Copyright Amazon.com Inc. 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 + * 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. + */ + +package compiler.c2.loopopts; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; +import jdk.test.lib.Utils; +import java.util.Random; + +/* + * @test + * @bug 8323220 + * @summary Test loop invariant code motion of add/sub through reassociation + * @library /test/lib / + * @run driver compiler.c2.loopopts.InvariantCodeMotionReassociateAddSub + */ +public class InvariantCodeMotionReassociateAddSub { + private static final Random RANDOM = Utils.getRandomInstance(); + private int size; + private int inv1; + private int inv2; + + public static void main(String[] args) { + TestFramework.run(); + } + + @DontInline + private int blackhole(int i) { return i; } + @DontInline + private int blackhole(long i) { return (int)i; } + + @Setup + public Object[] setup(SetupInfo info) { + int count = info.invocationCounter(); + size = count + 500; + inv1 = count; + if (RANDOM.nextInt() % 7 == 0) { + // Setup inputs to be equals sometimes to avoid uncommon traps + inv2 = inv1; + } else { + inv2 = count * 2; + } + return new Object[] { inv1, inv2, size }; + } + + public void fail(int returnValue) { + throw new RuntimeException("Illegal reassociation: returnValue=" + returnValue + ", inv1=" + inv1 + + ", inv2=" + inv2 + ", size=" + size); + } + + public void check(int returnValue, int expected) { + if (returnValue != expected) { + fail(returnValue); + } + } + + public void checkAdd(int returnValue) { + check(returnValue, inv1 + inv2 + size - 1); + } + + public void checkSubAdd(int returnValue) { + check(returnValue, inv1 - inv2 + size - 1); + } + + public void checkNegSubAdd(int returnValue) { + check(returnValue, -inv1 - inv2 + size - 1); + } + + public void checkAddSub(int returnValue) { + check(returnValue, inv1 + inv2 - (size - 1)); + } + + public void checkSubSub(int returnValue) { + check(returnValue, inv1 - inv2 - (size - 1)); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "3"}) + public int addInt(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 + i` + result = blackhole(inv1 + i + inv2); + } + return result; + } + + @Check(test = "addInt") + public void checkAddInt(int returnValue) { + checkAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "2"}) + public int addLong(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 + i` + result = blackhole(inv1 + i + inv2); + } + return result; + } + + @Check(test = "addLong") + public void checkAddLong(int returnValue) { + checkAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "3"}) + public int addInt2(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 + i` + result = blackhole(inv1 + (i + inv2)); + } + return result; + } + + @Check(test = "addInt2") + public void checkAddInt2(int returnValue) { + checkAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "2"}) + public int addLong2(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 + i` + result = blackhole(inv1 + (i + inv2)); + } + return result; + } + + @Check(test = "addLong2") + public void checkAddLong2(int returnValue) { + checkAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "2"}) + @IR(counts = {IRNode.SUB_I, "1"}) + public int minusAddInt(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 + i` + result = blackhole(inv1 + (i - inv2)); + } + return result; + } + + @Check(test = "minusAddInt") + public void checkSubAddInt(int returnValue) { + checkSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "1"}) + @IR(counts = {IRNode.SUB_L, "1"}) + public int minusAddLong(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 + i` + result = blackhole(inv1 + (i - inv2)); + } + return result; + } + + @Check(test = "minusAddLong") + public void checkSubAddLong(int returnValue) { + checkSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "2"}) + @IR(counts = {IRNode.SUB_I, "1"}) + public int minusAddInt2(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 + i` + result = blackhole(inv1 - (inv2 - i)); + } + return result; + } + + @Check(test = "minusAddInt2") + public void checkSubAddInt2(int returnValue) { + checkSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "1"}) + @IR(counts = {IRNode.SUB_L, "1"}) + public int minusAddLong2(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 + i` + result = blackhole(inv1 - (inv2 - i)); + } + return result; + } + + @Check(test = "minusAddLong2") + public void checkSubAddLong2(int returnValue) { + checkSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "2"}) + @IR(counts = {IRNode.SUB_I, "1"}) + public int minusAddInt3(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 + i` + result = blackhole(i - inv2 + inv1); + } + return result; + } + + @Check(test = "minusAddInt3") + public void checkSubAddInt3(int returnValue) { + checkSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "1"}) + @IR(counts = {IRNode.SUB_L, "1"}) + public int minusAddLong3(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 + i` + result = blackhole(i - inv2 + inv1); + } + return result; + } + + @Check(test = "minusAddLong3") + public void checkSubAddLong3(int returnValue) { + checkSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "2"}) + @IR(counts = {IRNode.SUB_I, "1"}) + public int negAddInt(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `-inv2 + inv1 + i` + result = blackhole(i + inv1 - inv2); + } + return result; + } + + @Check(test = "negAddInt") + public void checkNegAddInt(int returnValue) { + checkSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "1"}) + @IR(counts = {IRNode.SUB_L, "1"}) + public int negAddLong(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `-inv2 + inv1 + i` + result = blackhole(i + inv1 - inv2); + } + return result; + } + + @Check(test = "negAddLong") + public void checkNegAddLong(int returnValue) { + checkSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "2"}) + @IR(counts = {IRNode.SUB_I, "2"}) + public int negSubAddInt(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `-inv1 - inv2 + i` + result = blackhole(i - inv1 - inv2); + } + return result; + } + + @Check(test = "negSubAddInt") + public void checkNegSubAddInt(int returnValue) { + checkNegSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "1"}) + @IR(counts = {IRNode.SUB_L, "2"}) + public int negSubAddLong(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `-inv1 - inv2 + i` + result = blackhole(i - inv1 - inv2); + } + return result; + } + + @Check(test = "negSubAddLong") + public void checkNegSubAddLong(int returnValue) { + checkNegSubAdd(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "3"}) + @IR(counts = {IRNode.SUB_I, "1"}) + public int addSubInt(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 - i` + result = blackhole(inv1 + (inv2 - i)); + } + return result; + } + + @Check(test = "addSubInt") + public void checkAddSubInt(int returnValue) { + checkAddSub(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "1"}) + @IR(counts = {IRNode.SUB_L, "1"}) + public int addSubLong(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 - i` + result = blackhole(inv1 + (inv2 - i)); + } + return result; + } + + @Check(test = "addSubLong") + public void checkAddSubLong(int returnValue) { + checkAddSub(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "3"}) + @IR(counts = {IRNode.SUB_I, "1"}) + public int addSubInt2(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 - i` + result = blackhole(inv1 - (i - inv2)); + } + return result; + } + + @Check(test = "addSubInt2") + public void checkAddSubInt2(int returnValue) { + checkAddSub(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "1"}) + @IR(counts = {IRNode.SUB_L, "1"}) + public int addSubLong2(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 - i` + result = blackhole(inv1 - (i - inv2)); + } + return result; + } + + @Check(test = "addSubLong2") + public void checkAddSubLong2(int returnValue) { + checkAddSub(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "3"}) + @IR(counts = {IRNode.SUB_I, "1"}) + public int addSubInt3(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 - i` + result = blackhole(inv2 - i + inv1); + } + return result; + } + + @Check(test = "addSubInt3") + public void checkAddSubInt3(int returnValue) { + checkAddSub(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_L, "1"}) + @IR(counts = {IRNode.SUB_L, "1"}) + public int addSubLong3(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 + inv2 - i` + result = blackhole(inv2 - i + inv1); + } + return result; + } + + @Check(test = "addSubLong3") + public void checkAddSubLong3(int returnValue) { + checkAddSub(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "2"}) + @IR(counts = {IRNode.SUB_I, "2"}) + public int subSubInt(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 - i` + result = blackhole(inv1 - (i + inv2)); + } + return result; + } + + @Check(test = "subSubInt") + public void checkSubSubInt(int returnValue) { + checkSubSub(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(failOn = {IRNode.ADD_L}) + @IR(counts = {IRNode.SUB_L, "2"}) + public int subSubLong(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 - i` + result = blackhole(inv1 - (i + inv2)); + } + return result; + } + + @Check(test = "subSubLong") + public void checkSubSubLong(int returnValue) { + checkSubSub(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(counts = {IRNode.ADD_I, "2"}) + @IR(counts = {IRNode.SUB_I, "2"}) + public int subSubInt2(int inv1, int inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 - i` + result = blackhole(inv1 - i - inv2); + } + return result; + } + + @Check(test = "subSubInt2") + public void checkSubSubInt2(int returnValue) { + checkSubSub(returnValue); + } + + @Test + @Arguments(setup = "setup") + @IR(failOn = {IRNode.ADD_L}) + @IR(counts = {IRNode.SUB_L, "2"}) + public int subSubLong2(long inv1, long inv2, int size) { + int result = -1; + for (int i = 0; i < size; ++i) { + // Reassociate to `inv1 - inv2 - i` + result = blackhole(inv1 - i - inv2); + } + return result; + } + + @Check(test = "subSubLong2") + public void checkSubSubLong2(int returnValue) { + checkSubSub(returnValue); + } +} diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index 05707820bcb93..566e294b4bffd 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -63,8 +63,8 @@ public Object[] setup(SetupInfo info) { } public void fail(int returnValue) { - throw new RuntimeException("Illegal reassociation: i=" + returnValue + ", inv1=" + inv1 - + ", inv2=" + inv2); + throw new RuntimeException("Illegal reassociation: returnValue=" + returnValue + ", inv1=" + inv1 + + ", inv2=" + inv2 + ", size=" + size); } public void checkEq(int returnValue) { From 33e34b0300664ed84a7511e3f943e9a01a499d84 Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Thu, 21 Mar 2024 19:35:34 +0100 Subject: [PATCH 13/16] @run driver -> @run main --- .../compiler/loopopts/InvariantCodeMotionReassociateAddSub.java | 2 +- .../compiler/loopopts/InvariantCodeMotionReassociateCmp.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java index 4e8daf3d163c6..ce223b88da4f9 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java @@ -33,7 +33,7 @@ * @bug 8323220 * @summary Test loop invariant code motion of add/sub through reassociation * @library /test/lib / - * @run driver compiler.c2.loopopts.InvariantCodeMotionReassociateAddSub + * @run main compiler.c2.loopopts.InvariantCodeMotionReassociateAddSub */ public class InvariantCodeMotionReassociateAddSub { private static final Random RANDOM = Utils.getRandomInstance(); diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index 566e294b4bffd..ce99b9aed28c1 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -33,7 +33,7 @@ * @bug 8323220 * @summary Test loop invariant code motion for cmp nodes through reassociation * @library /test/lib / - * @run driver compiler.c2.loopopts.InvariantCodeMotionReassociateCmp + * @run main compiler.c2.loopopts.InvariantCodeMotionReassociateCmp */ public class InvariantCodeMotionReassociateCmp { private static final Random RANDOM = Utils.getRandomInstance(); From d5cd233942ddc7366cb8cb27be02368cb3d0bc14 Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Fri, 5 Apr 2024 19:31:12 +0200 Subject: [PATCH 14/16] Formatting, use @run driver, remove legacy header comments --- src/hotspot/share/opto/loopTransform.cpp | 21 +++++++------------ src/hotspot/share/opto/loopnode.hpp | 2 +- .../InvariantCodeMotionReassociateAddSub.java | 2 +- .../InvariantCodeMotionReassociateCmp.java | 2 +- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index a1ac544620c6d..9174bd9016f48 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -254,7 +254,6 @@ void IdealLoopTree::compute_profile_trip_cnt(PhaseIdealLoop *phase) { head->set_profile_trip_cnt(trip_cnt); } -//---------------------find_invariant----------------------------- // Return nonzero index of invariant operand for an associative // binary operation of (nonconstant) invariant and variant values. // Helper for reassociate_invariants. @@ -266,7 +265,6 @@ int IdealLoopTree::find_invariant(Node* n, PhaseIdealLoop* phase) { return 0; } -//---------------------is_associative_cmp------------------------- // Return TRUE if "n" is an associative cmp node. A cmp node is // associative if it is only used for equals or not-equals // comparisons of integers or longs. We cannot reassociate @@ -276,16 +274,15 @@ bool IdealLoopTree::is_associative_cmp(Node* n) { return false; } for (DUIterator i = n->outs(); n->has_out(i); i++) { - BoolNode* boolOut = n->out(i)->isa_Bool(); - if (boolOut == nullptr || !(boolOut->_test._test == BoolTest::eq || - boolOut->_test._test == BoolTest::ne)) { + BoolNode *bool_out = n->out(i)->isa_Bool(); + if (bool_out == nullptr || !(bool_out->_test._test == BoolTest::eq || + bool_out->_test._test == BoolTest::ne)) { return false; } } return true; } -//---------------------is_associative----------------------------- // Return TRUE if "n" is an associative binary node. If "base" is // not null, "n" must be re-associative with it. bool IdealLoopTree::is_associative(Node* n, Node* base) { @@ -301,7 +298,8 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { } return op == base_op; } else { - // Integer "add/sub/mul/and/or/xor" operations are associative. + // Integer "add/sub/mul/and/or/xor" operations are associative. Integer + // "cmp" operations are virtual if it is an equality comparison. return op == Op_AddI || op == Op_AddL || op == Op_SubI || op == Op_SubL || op == Op_MulI || op == Op_MulL @@ -312,7 +310,6 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { } } -//-------------------reassociate_add_sub_cmp--------------------- // Reassociate invariant add and subtract expressions: // // inv1 + (x + inv2) => ( inv1 + inv2) + x @@ -332,7 +329,6 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { // inv1 == (x + inv2) => ( inv1 - inv2 ) == x // inv1 == (x - inv2) => ( inv1 + inv2 ) == x // inv1 == (inv2 - x) => (-inv1 + inv2 ) == x -// Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop* phase) { Node* n2 = n1->in(3 - inv1_idx); bool n1_is_sub = n1->is_Sub() && !n1->is_Cmp(); @@ -347,10 +343,8 @@ Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_id // Determine whether x, inv1, or inv2 should be negative in the transformed // expression bool neg_x = n2_is_sub && inv2_idx == 1; - bool neg_inv2 = - (n2_is_sub && !n1_is_cmp && inv2_idx == 2) || (n1_is_cmp && !n2_is_sub); - bool neg_inv1 = - (n1_is_sub && inv1_idx == 2) || (n1_is_cmp && inv2_idx == 1 && n2_is_sub); + bool neg_inv2 = (n2_is_sub && !n1_is_cmp && inv2_idx == 2) || (n1_is_cmp && !n2_is_sub); + bool neg_inv1 = (n1_is_sub && inv1_idx == 2) || (n1_is_cmp && inv2_idx == 1 && n2_is_sub); if (n1_is_sub && inv1_idx == 1) { neg_x = !neg_x; neg_inv2 = !neg_inv2; @@ -408,7 +402,6 @@ Node* IdealLoopTree::reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_id } } -//---------------------reassociate----------------------------- // Reassociate invariant binary expressions with add/sub/mul/ // and/or/xor/cmp operators. // For add/sub/cmp expressions: see "reassociate_add_sub_cmp" diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index ec86ac7104f5c..aa7a2cc07d10f 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -740,7 +740,7 @@ class IdealLoopTree : public ResourceObj { // Reassociate invariant binary expressions. Node* reassociate(Node* n1, PhaseIdealLoop *phase); // Reassociate invariant add, subtract, and compare expressions. - Node* reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop *phase); + Node* reassociate_add_sub_cmp(Node* n1, int inv1_idx, int inv2_idx, PhaseIdealLoop* phase); // Return nonzero index of invariant operand if invariant and variant // are combined with an associative binary. Helper for reassociate_invariants. int find_invariant(Node* n, PhaseIdealLoop *phase); diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java index ce223b88da4f9..4e8daf3d163c6 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateAddSub.java @@ -33,7 +33,7 @@ * @bug 8323220 * @summary Test loop invariant code motion of add/sub through reassociation * @library /test/lib / - * @run main compiler.c2.loopopts.InvariantCodeMotionReassociateAddSub + * @run driver compiler.c2.loopopts.InvariantCodeMotionReassociateAddSub */ public class InvariantCodeMotionReassociateAddSub { private static final Random RANDOM = Utils.getRandomInstance(); diff --git a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java index ce99b9aed28c1..566e294b4bffd 100644 --- a/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java +++ b/test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java @@ -33,7 +33,7 @@ * @bug 8323220 * @summary Test loop invariant code motion for cmp nodes through reassociation * @library /test/lib / - * @run main compiler.c2.loopopts.InvariantCodeMotionReassociateCmp + * @run driver compiler.c2.loopopts.InvariantCodeMotionReassociateCmp */ public class InvariantCodeMotionReassociateCmp { private static final Random RANDOM = Utils.getRandomInstance(); From 1b27aae4c086da622b5311ae227e59cf582cf85f Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Fri, 5 Apr 2024 19:33:56 +0200 Subject: [PATCH 15/16] Fix typo --- src/hotspot/share/opto/loopTransform.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 9174bd9016f48..070320f552453 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -299,7 +299,7 @@ bool IdealLoopTree::is_associative(Node* n, Node* base) { return op == base_op; } else { // Integer "add/sub/mul/and/or/xor" operations are associative. Integer - // "cmp" operations are virtual if it is an equality comparison. + // "cmp" operations are associative if it is an equality comparison. return op == Op_AddI || op == Op_AddL || op == Op_SubI || op == Op_SubL || op == Op_MulI || op == Op_MulL From 3bcab3bb4a6a3a6f421371a3e91de52ce98f1def Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Mon, 8 Apr 2024 11:38:29 -0700 Subject: [PATCH 16/16] Update src/hotspot/share/opto/loopTransform.cpp formatting Co-authored-by: Christian Hagedorn --- src/hotspot/share/opto/loopTransform.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 070320f552453..0040a2b0b503d 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -274,7 +274,7 @@ bool IdealLoopTree::is_associative_cmp(Node* n) { return false; } for (DUIterator i = n->outs(); n->has_out(i); i++) { - BoolNode *bool_out = n->out(i)->isa_Bool(); + BoolNode* bool_out = n->out(i)->isa_Bool(); if (bool_out == nullptr || !(bool_out->_test._test == BoolTest::eq || bool_out->_test._test == BoolTest::ne)) { return false;