From bd77da8bf9889f00b7e2055c0020148ab5d60e31 Mon Sep 17 00:00:00 2001 From: Matthias Ernst Date: Sat, 21 Dec 2024 14:59:03 +0100 Subject: [PATCH 1/7] C2: optimize constant addends in masked sums Extends the optimization of masked sums introduced in #6697 to cover constant values, which currently break the optimization. Such constant values arise in an expression of the following form, for example from MemorySegmentImpl#isAlignedForElement: (base + (index + 1) << 8) & 255 => MulNode (base + (index << 8 + 256)) & 255 => AddNode ((base + index << 8) + 256) & 255 Currently, "256" is not being recognized as a shifted value. This PR enables: ((base + index << 8) + 256) & 255 => MulNode (base + index << 8) & 255 => MulNode (PR #6697) base & 255 --- src/hotspot/share/opto/mulnode.cpp | 144 +++++++++--------- src/hotspot/share/opto/mulnode.hpp | 5 +- .../compiler/c2/irTests/TestShiftAndMask.java | 4 +- 3 files changed, 74 insertions(+), 79 deletions(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index ad98fda025f07..d8dfa7bade69b 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -672,7 +672,7 @@ const Type *AndINode::mul_ring( const Type *t0, const Type *t1 ) const { const Type* AndINode::Value(PhaseGVN* phase) const { // patterns similar to (v << 2) & 3 - if (AndIL_shift_and_mask_is_always_zero(phase, in(1), in(2), T_INT, true)) { + if (AndIL_is_always_zero(phase, in(1), in(2), T_INT, true)) { return TypeInt::ZERO; } @@ -719,7 +719,7 @@ Node* AndINode::Identity(PhaseGVN* phase) { //------------------------------Ideal------------------------------------------ Node *AndINode::Ideal(PhaseGVN *phase, bool can_reshape) { // pattern similar to (v1 + (v2 << 2)) & 3 transformed to v1 & 3 - Node* progress = AndIL_add_shift_and_mask(phase, T_INT); + Node* progress = AndIL_sum_and_mask(phase, T_INT); if (progress != nullptr) { return progress; } @@ -803,7 +803,7 @@ const Type *AndLNode::mul_ring( const Type *t0, const Type *t1 ) const { const Type* AndLNode::Value(PhaseGVN* phase) const { // patterns similar to (v << 2) & 3 - if (AndIL_shift_and_mask_is_always_zero(phase, in(1), in(2), T_LONG, true)) { + if (AndIL_is_always_zero(phase, in(1), in(2), T_LONG, true)) { return TypeLong::ZERO; } @@ -851,7 +851,7 @@ Node* AndLNode::Identity(PhaseGVN* phase) { //------------------------------Ideal------------------------------------------ Node *AndLNode::Ideal(PhaseGVN *phase, bool can_reshape) { // pattern similar to (v1 + (v2 << 2)) & 3 transformed to v1 & 3 - Node* progress = AndIL_add_shift_and_mask(phase, T_LONG); + Node* progress = AndIL_sum_and_mask(phase, T_LONG); if (progress != nullptr) { return progress; } @@ -2052,9 +2052,52 @@ const Type* RotateRightNode::Value(PhaseGVN* phase) const { } } -// Given an expression (AndX shift mask) or (AndX mask shift), +// Returns a lower bound of the number of trailing zeros in expr. +jint MulNode::AndIL_min_trailing_zeros(PhaseGVN* phase, Node* expr, BasicType bt) { + expr = expr->uncast(); + if (expr == nullptr) { + return 0; + } + const TypeInteger* type = phase->type(expr)->isa_integer(bt); + if (type == nullptr) { + return 0; + } + + if (type->is_con()) { + long con = type->get_con_as_long(type->basic_type()); + return con == 0L ? 0 : count_trailing_zeros(con); + } + + if (expr->Opcode() == Op_ConvI2L) { + expr = expr->in(1); + if (expr == nullptr) { + return 0; + } + expr = expr->uncast(); + if (expr == nullptr) { + return 0; + } + type = phase->type(expr)->isa_int(); + } + + if (expr->Opcode() == Op_LShift(type->basic_type())) { + Node* rhs = expr->in(2); + if (rhs == nullptr) { + return 0; + } + const TypeInt* rhs_t = phase->type(rhs)->isa_int(); + if (!rhs_t || !rhs_t->is_con()) { + return 0; + } + return rhs_t->get_con() & ((type->isa_int() ? BitsPerJavaInteger : BitsPerJavaLong) - 1); + } + + return 0; +} + +// Given an expression (AndX expr mask) or (AndX mask expr), // determine if the AndX must always produce zero, because the -// the shift (x< #0 // (AndL (LShiftL _ #N) #M) => #0 // (AndL (ConvI2L (LShiftI _ #N)) #M) => #0 +// as well as for constant operands: +// (AndI (ConI [+-] _ << #N) #M) => #0 +// (AndL (ConL [+-] _ << #N) #M) => #0 // The M and N values must satisfy ((-1 << N) & M) == 0. // Because the optimization might work for a non-constant -// mask M, we check the AndX for both operand orders. -bool MulNode::AndIL_shift_and_mask_is_always_zero(PhaseGVN* phase, Node* shift, Node* mask, BasicType bt, bool check_reverse) { - if (mask == nullptr || shift == nullptr) { +// mask M, we check for both operand orders. +bool MulNode::AndIL_is_always_zero(PhaseGVN* phase, Node* expr, Node* mask, BasicType bt, bool check_reverse) { + if (mask == nullptr || expr == nullptr) { return false; } const TypeInteger* mask_t = phase->type(mask)->isa_integer(bt); - if (mask_t == nullptr || phase->type(shift)->isa_integer(bt) == nullptr) { + if (mask_t == nullptr) { return false; } - shift = shift->uncast(); - if (shift == nullptr) { - return false; - } - if (phase->type(shift)->isa_integer(bt) == nullptr) { - return false; - } - BasicType shift_bt = bt; - if (bt == T_LONG && shift->Opcode() == Op_ConvI2L) { - bt = T_INT; - Node* val = shift->in(1); - if (val == nullptr) { - return false; - } - val = val->uncast(); - if (val == nullptr) { - return false; - } - if (val->Opcode() == Op_LShiftI) { - shift_bt = T_INT; - shift = val; - if (phase->type(shift)->isa_integer(bt) == nullptr) { - return false; - } - } - } - if (shift->Opcode() != Op_LShift(shift_bt)) { - if (check_reverse && - (mask->Opcode() == Op_LShift(bt) || - (bt == T_LONG && mask->Opcode() == Op_ConvI2L))) { - // try it the other way around - return AndIL_shift_and_mask_is_always_zero(phase, mask, shift, bt, false); - } - return false; - } - Node* shift2 = shift->in(2); - if (shift2 == nullptr) { - return false; - } - const Type* shift2_t = phase->type(shift2); - if (!shift2_t->isa_int() || !shift2_t->is_int()->is_con()) { - return false; - } - - jint shift_con = shift2_t->is_int()->get_con() & ((shift_bt == T_INT ? BitsPerJavaInteger : BitsPerJavaLong) - 1); - if ((((jlong)1) << shift_con) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0) { - return true; + jint zeros = AndIL_min_trailing_zeros(phase, expr, bt); + if (zeros == 0) { + // try it the other way around + return check_reverse && AndIL_is_always_zero(phase, mask, expr, bt, false); } - return false; + return ((((jlong)1) << zeros) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0); } -// Given an expression (AndX (AddX v1 (LShiftX v2 #N)) #M) -// determine if the AndX must always produce (AndX v1 #M), -// because the shift (v2< (AndI v1 #M) -// (AndL (AddI v1 (LShiftL _ #N)) #M) => (AndL v1 #M) -// (AndL (AddL v1 (ConvI2L (LShiftI _ #N))) #M) => (AndL v1 #M) -// The M and N values must satisfy ((-1 << N) & M) == 0. -// Because the optimization might work for a non-constant -// mask M, and because the AddX operands can come in either -// order, we check for every operand order. -Node* MulNode::AndIL_add_shift_and_mask(PhaseGVN* phase, BasicType bt) { +// Given an expression (AndX (AddX v1 v2) mask) +// determine if the AndX must always produce (AndX v1 mask), +// because v2 is bitwise disjoint from the mask. +// Because the AddX operands can come in either +// order, we check for both orders. +Node* MulNode::AndIL_sum_and_mask(PhaseGVN* phase, BasicType bt) { Node* add = in(1); Node* mask = in(2); if (add == nullptr || mask == nullptr) { @@ -2157,10 +2151,10 @@ Node* MulNode::AndIL_add_shift_and_mask(PhaseGVN* phase, BasicType bt) { Node* add1 = add->in(1); Node* add2 = add->in(2); if (add1 != nullptr && add2 != nullptr) { - if (AndIL_shift_and_mask_is_always_zero(phase, add1, mask, bt, false)) { + if (AndIL_is_always_zero(phase, add1, mask, bt, false)) { set_req_X(addidx, add2, phase); return this; - } else if (AndIL_shift_and_mask_is_always_zero(phase, add2, mask, bt, false)) { + } else if (AndIL_is_always_zero(phase, add2, mask, bt, false)) { set_req_X(addidx, add1, phase); return this; } diff --git a/src/hotspot/share/opto/mulnode.hpp b/src/hotspot/share/opto/mulnode.hpp index c8d168685d9e5..ffdf8ca0bf422 100644 --- a/src/hotspot/share/opto/mulnode.hpp +++ b/src/hotspot/share/opto/mulnode.hpp @@ -83,8 +83,9 @@ class MulNode : public Node { static MulNode* make(Node* in1, Node* in2, BasicType bt); - static bool AndIL_shift_and_mask_is_always_zero(PhaseGVN* phase, Node* shift, Node* mask, BasicType bt, bool check_reverse); - Node* AndIL_add_shift_and_mask(PhaseGVN* phase, BasicType bt); + static jint AndIL_min_trailing_zeros(PhaseGVN* phase, Node* addend, BasicType bt); + static bool AndIL_is_always_zero(PhaseGVN* phase, Node* expr, Node* mask, BasicType bt, bool check_reverse); + Node* AndIL_sum_and_mask(PhaseGVN* phase, BasicType bt); }; //------------------------------MulINode--------------------------------------- diff --git a/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java b/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java index 4396873425ae1..0b056e6223819 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java @@ -120,7 +120,7 @@ public static void checkShiftNonConstMaskLong(long res) { @IR(counts = { IRNode.AND_I, "1" }) @IR(failOn = { IRNode.ADD_I, IRNode.LSHIFT_I }) public static int addShiftMaskInt(int i, int j) { - return (j + (i << 2)) & 3; // transformed to: return j & 3; + return (j + ((i + 1) << 2)) & 3; // transformed to: return j & 3; } @Run(test = "addShiftMaskInt") @@ -165,7 +165,7 @@ public static void addSshiftNonConstMaskInt_runner() { @IR(counts = { IRNode.AND_L, "1" }) @IR(failOn = { IRNode.ADD_L, IRNode.LSHIFT_L }) public static long addShiftMaskLong(long i, long j) { - return (j + (i << 2)) & 3; // transformed to: return j & 3; + return (j + ((i - 3) << 2)) & 3; // transformed to: return j & 3; } @Run(test = "addShiftMaskLong") From 2b8da62aa217ac13fa7d9cc3a84912430834f4c1 Mon Sep 17 00:00:00 2001 From: Matthias Ernst Date: Tue, 24 Dec 2024 09:03:44 +0100 Subject: [PATCH 2/7] review: * make helpers static * remove null checks * separate test cases * naming / reword comments readability: * move reverse checks to (some) callsites --- src/hotspot/share/opto/mulnode.cpp | 131 ++++++++---------- src/hotspot/share/opto/mulnode.hpp | 3 +- .../compiler/c2/irTests/TestShiftAndMask.java | 40 +++++- 3 files changed, 95 insertions(+), 79 deletions(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index d8dfa7bade69b..ed6df4a3ed8b4 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -670,9 +670,13 @@ const Type *AndINode::mul_ring( const Type *t0, const Type *t1 ) const { return and_value(r0, r1); } +// Is expr a neutral element wrt addition under mask? +static bool AndIL_is_zero_element(const PhaseGVN* phase, const Node* expr, const Node* mask, BasicType bt); + const Type* AndINode::Value(PhaseGVN* phase) const { // patterns similar to (v << 2) & 3 - if (AndIL_is_always_zero(phase, in(1), in(2), T_INT, true)) { + if (AndIL_is_zero_element(phase, in(1), in(2), T_INT) || + AndIL_is_zero_element(phase, in(2), in(1), T_INT)) { return TypeInt::ZERO; } @@ -803,7 +807,8 @@ const Type *AndLNode::mul_ring( const Type *t0, const Type *t1 ) const { const Type* AndLNode::Value(PhaseGVN* phase) const { // patterns similar to (v << 2) & 3 - if (AndIL_is_always_zero(phase, in(1), in(2), T_LONG, true)) { + if (AndIL_is_zero_element(phase, in(1), in(2), T_LONG) || + AndIL_is_zero_element(phase, in(2), in(1), T_LONG)) { return TypeLong::ZERO; } @@ -2052,12 +2057,44 @@ const Type* RotateRightNode::Value(PhaseGVN* phase) const { } } -// Returns a lower bound of the number of trailing zeros in expr. -jint MulNode::AndIL_min_trailing_zeros(PhaseGVN* phase, Node* expr, BasicType bt) { - expr = expr->uncast(); - if (expr == nullptr) { - return 0; +// Given an expression (AndX (AddX v1 v2) mask) +// determine if the AndX must always produce (AndX v1 mask), +// because v2 is zero wrt addition under mask. +// Because the AddX operands can come in either +// order, we check for both orders. +Node* MulNode::AndIL_sum_and_mask(PhaseGVN* phase, BasicType bt) { + Node* add = in(1); + Node* mask = in(2); + if (add == nullptr || mask == nullptr) { + return nullptr; + } + int addidx = 0; + if (add->Opcode() == Op_Add(bt)) { + addidx = 1; + } else if (mask->Opcode() == Op_Add(bt)) { + mask = add; + addidx = 2; + add = in(addidx); } + if (addidx > 0) { + Node* add1 = add->in(1); + Node* add2 = add->in(2); + if (add1 != nullptr && add2 != nullptr) { + if (AndIL_is_zero_element(phase, add1, mask, bt)) { + set_req_X(addidx, add2, phase); + return this; + } else if (AndIL_is_zero_element(phase, add2, mask, bt)) { + set_req_X(addidx, add1, phase); + return this; + } + } + } + return nullptr; +} + +// Returns a lower bound on the number of trailing zeros in expr. +static jint AndIL_min_trailing_zeros(const PhaseGVN* phase, const Node* expr, BasicType bt) { + expr = expr->uncast(); const TypeInteger* type = phase->type(expr)->isa_integer(bt); if (type == nullptr) { return 0; @@ -2065,41 +2102,31 @@ jint MulNode::AndIL_min_trailing_zeros(PhaseGVN* phase, Node* expr, BasicType bt if (type->is_con()) { long con = type->get_con_as_long(type->basic_type()); - return con == 0L ? 0 : count_trailing_zeros(con); + return con == 0L ? (type2aelembytes(bt) * BitsPerByte) : count_trailing_zeros(con); } if (expr->Opcode() == Op_ConvI2L) { - expr = expr->in(1); - if (expr == nullptr) { - return 0; - } - expr = expr->uncast(); - if (expr == nullptr) { - return 0; - } + expr = expr->in(1)->uncast(); + bt = T_INT; type = phase->type(expr)->isa_int(); } if (expr->Opcode() == Op_LShift(type->basic_type())) { - Node* rhs = expr->in(2); - if (rhs == nullptr) { + const TypeInt* rhs_t = phase->type(expr->in(2))->isa_int(); + if (rhs_t == nullptr || !rhs_t->is_con()) { return 0; } - const TypeInt* rhs_t = phase->type(rhs)->isa_int(); - if (!rhs_t || !rhs_t->is_con()) { - return 0; - } - return rhs_t->get_con() & ((type->isa_int() ? BitsPerJavaInteger : BitsPerJavaLong) - 1); + return rhs_t->get_con() % (type2aelembytes(bt) * BitsPerByte); } return 0; } -// Given an expression (AndX expr mask) or (AndX mask expr), -// determine if the AndX must always produce zero, because the -// expr is bitwise disjoint from the mask. +// Given an expression (AndX X+expr mask), determine +// whether expr is neutral wrt addition under mask +// and hence the result is always equivalent to (AndX X mask), // The X in AndX must be I or L, depending on bt. -// Specifically, the following cases fold to zero, +// Specifically, this holds for the following cases, // when the shift value N is large enough to zero out // all the set positions of the and-mask M. // (AndI (LShiftI _ #N) #M) => #0 @@ -2109,56 +2136,12 @@ jint MulNode::AndIL_min_trailing_zeros(PhaseGVN* phase, Node* expr, BasicType bt // (AndI (ConI [+-] _ << #N) #M) => #0 // (AndL (ConL [+-] _ << #N) #M) => #0 // The M and N values must satisfy ((-1 << N) & M) == 0. -// Because the optimization might work for a non-constant -// mask M, we check for both operand orders. -bool MulNode::AndIL_is_always_zero(PhaseGVN* phase, Node* expr, Node* mask, BasicType bt, bool check_reverse) { - if (mask == nullptr || expr == nullptr) { - return false; - } +static bool AndIL_is_zero_element(const PhaseGVN* phase, const Node* expr, const Node* mask, BasicType bt) { const TypeInteger* mask_t = phase->type(mask)->isa_integer(bt); if (mask_t == nullptr) { return false; } - jint zeros = AndIL_min_trailing_zeros(phase, expr, bt); - if (zeros == 0) { - // try it the other way around - return check_reverse && AndIL_is_always_zero(phase, mask, expr, bt, false); - } - - return ((((jlong)1) << zeros) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0); -} -// Given an expression (AndX (AddX v1 v2) mask) -// determine if the AndX must always produce (AndX v1 mask), -// because v2 is bitwise disjoint from the mask. -// Because the AddX operands can come in either -// order, we check for both orders. -Node* MulNode::AndIL_sum_and_mask(PhaseGVN* phase, BasicType bt) { - Node* add = in(1); - Node* mask = in(2); - if (add == nullptr || mask == nullptr) { - return nullptr; - } - int addidx = 0; - if (add->Opcode() == Op_Add(bt)) { - addidx = 1; - } else if (mask->Opcode() == Op_Add(bt)) { - mask = add; - addidx = 2; - add = in(addidx); - } - if (addidx > 0) { - Node* add1 = add->in(1); - Node* add2 = add->in(2); - if (add1 != nullptr && add2 != nullptr) { - if (AndIL_is_always_zero(phase, add1, mask, bt, false)) { - set_req_X(addidx, add2, phase); - return this; - } else if (AndIL_is_always_zero(phase, add2, mask, bt, false)) { - set_req_X(addidx, add1, phase); - return this; - } - } - } - return nullptr; + jint zeros = AndIL_min_trailing_zeros(phase, expr, bt); + return zeros > 0 && ((((jlong)1) << zeros) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0); } diff --git a/src/hotspot/share/opto/mulnode.hpp b/src/hotspot/share/opto/mulnode.hpp index ffdf8ca0bf422..0eec6dd10c019 100644 --- a/src/hotspot/share/opto/mulnode.hpp +++ b/src/hotspot/share/opto/mulnode.hpp @@ -83,8 +83,7 @@ class MulNode : public Node { static MulNode* make(Node* in1, Node* in2, BasicType bt); - static jint AndIL_min_trailing_zeros(PhaseGVN* phase, Node* addend, BasicType bt); - static bool AndIL_is_always_zero(PhaseGVN* phase, Node* expr, Node* mask, BasicType bt, bool check_reverse); +protected: Node* AndIL_sum_and_mask(PhaseGVN* phase, BasicType bt); }; diff --git a/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java b/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java index 0b056e6223819..098617d29e546 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java @@ -30,7 +30,7 @@ /* * @test - * @bug 8277850 8278949 8285793 + * @bug 8277850 8278949 8285793 8346664 * @summary C2: optimize mask checks in counted loops * @library /test/lib / * @run driver compiler.c2.irTests.TestShiftAndMask @@ -120,7 +120,7 @@ public static void checkShiftNonConstMaskLong(long res) { @IR(counts = { IRNode.AND_I, "1" }) @IR(failOn = { IRNode.ADD_I, IRNode.LSHIFT_I }) public static int addShiftMaskInt(int i, int j) { - return (j + ((i + 1) << 2)) & 3; // transformed to: return j & 3; + return (j + (i << 2)) & 3; // transformed to: return j & 3; } @Run(test = "addShiftMaskInt") @@ -133,6 +133,23 @@ public static void addShiftMaskInt_runner() { } } + @Test + @IR(counts = { IRNode.AND_I, "1" }) + @IR(failOn = { IRNode.ADD_I, IRNode.LSHIFT_I }) + public static int addShiftPlusConstMaskInt(int i, int j) { + return (j + ((i + 5) << 2)) & 3; // transformed to: return j & 3; + } + + @Run(test = "addShiftPlusConstMaskInt") + public static void addShiftPlusConstMaskInt_runner() { + int i = RANDOM.nextInt(); + int j = RANDOM.nextInt(); + int res = addShiftPlusConstMaskInt(i, j); + if (res != (j & 3)) { + throw new RuntimeException("incorrect result: " + res); + } + } + @Test @IR(counts = { IRNode.AND_I, "1" }) @IR(failOn = { IRNode.ADD_I, IRNode.LSHIFT_I }) @@ -165,7 +182,7 @@ public static void addSshiftNonConstMaskInt_runner() { @IR(counts = { IRNode.AND_L, "1" }) @IR(failOn = { IRNode.ADD_L, IRNode.LSHIFT_L }) public static long addShiftMaskLong(long i, long j) { - return (j + ((i - 3) << 2)) & 3; // transformed to: return j & 3; + return (j + (i << 2)) & 3; // transformed to: return j & 3; } @Run(test = "addShiftMaskLong") @@ -178,6 +195,23 @@ public static void addShiftMaskLong_runner() { } } + @Test + @IR(counts = { IRNode.AND_L, "1" }) + @IR(failOn = { IRNode.ADD_L, IRNode.LSHIFT_L }) + public static long addShiftPlusConstMaskLong(long i, long j) { + return (j + ((i - 5) << 2)) & 3; // transformed to: return j & 3; + } + + @Run(test = "addShiftPlusConstMaskLong") + public static void addShiftPlusConstMaskLong_runner() { + long i = RANDOM.nextLong(); + long j = RANDOM.nextLong(); + long res = addShiftPlusConstMaskLong(i, j); + if (res != (j & 3)) { + throw new RuntimeException("incorrect result: " + res); + } + } + @Test @IR(counts = { IRNode.AND_L, "1" }) @IR(failOn = { IRNode.ADD_L, IRNode.LSHIFT_L }) From bc3c4f03f3675ca23e8d3c3aa3a487e44c7b5e3d Mon Sep 17 00:00:00 2001 From: Matthias Ernst Date: Tue, 24 Dec 2024 09:29:44 +0100 Subject: [PATCH 3/7] undo method reordering --- src/hotspot/share/opto/mulnode.cpp | 70 +++++++++++++++--------------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index ed6df4a3ed8b4..9012a28615a4c 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -2057,40 +2057,7 @@ const Type* RotateRightNode::Value(PhaseGVN* phase) const { } } -// Given an expression (AndX (AddX v1 v2) mask) -// determine if the AndX must always produce (AndX v1 mask), -// because v2 is zero wrt addition under mask. -// Because the AddX operands can come in either -// order, we check for both orders. -Node* MulNode::AndIL_sum_and_mask(PhaseGVN* phase, BasicType bt) { - Node* add = in(1); - Node* mask = in(2); - if (add == nullptr || mask == nullptr) { - return nullptr; - } - int addidx = 0; - if (add->Opcode() == Op_Add(bt)) { - addidx = 1; - } else if (mask->Opcode() == Op_Add(bt)) { - mask = add; - addidx = 2; - add = in(addidx); - } - if (addidx > 0) { - Node* add1 = add->in(1); - Node* add2 = add->in(2); - if (add1 != nullptr && add2 != nullptr) { - if (AndIL_is_zero_element(phase, add1, mask, bt)) { - set_req_X(addidx, add2, phase); - return this; - } else if (AndIL_is_zero_element(phase, add2, mask, bt)) { - set_req_X(addidx, add1, phase); - return this; - } - } - } - return nullptr; -} +//------------------------------ Sum & Mask ------------------------------ // Returns a lower bound on the number of trailing zeros in expr. static jint AndIL_min_trailing_zeros(const PhaseGVN* phase, const Node* expr, BasicType bt) { @@ -2145,3 +2112,38 @@ static bool AndIL_is_zero_element(const PhaseGVN* phase, const Node* expr, const jint zeros = AndIL_min_trailing_zeros(phase, expr, bt); return zeros > 0 && ((((jlong)1) << zeros) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0); } + +// Given an expression (AndX (AddX v1 v2) mask) +// determine if the AndX must always produce (AndX v1 mask), +// because v2 is zero wrt addition under mask. +// Because the AddX operands can come in either +// order, we check for both orders. +Node* MulNode::AndIL_sum_and_mask(PhaseGVN* phase, BasicType bt) { + Node* add = in(1); + Node* mask = in(2); + if (add == nullptr || mask == nullptr) { + return nullptr; + } + int addidx = 0; + if (add->Opcode() == Op_Add(bt)) { + addidx = 1; + } else if (mask->Opcode() == Op_Add(bt)) { + mask = add; + addidx = 2; + add = in(addidx); + } + if (addidx > 0) { + Node* add1 = add->in(1); + Node* add2 = add->in(2); + if (add1 != nullptr && add2 != nullptr) { + if (AndIL_is_zero_element(phase, add1, mask, bt)) { + set_req_X(addidx, add2, phase); + return this; + } else if (AndIL_is_zero_element(phase, add2, mask, bt)) { + set_req_X(addidx, add1, phase); + return this; + } + } + } + return nullptr; +} From ba2f82b855bd9b1d70af3b89972f95a1486bcc5a Mon Sep 17 00:00:00 2001 From: Matthias Ernst Date: Tue, 24 Dec 2024 09:41:28 +0100 Subject: [PATCH 4/7] T != X --- src/hotspot/share/opto/mulnode.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index 9012a28615a4c..a43ba87c512ae 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -2089,9 +2089,9 @@ static jint AndIL_min_trailing_zeros(const PhaseGVN* phase, const Node* expr, Ba return 0; } -// Given an expression (AndX X+expr mask), determine +// Given an expression (AndX T+expr mask), determine // whether expr is neutral wrt addition under mask -// and hence the result is always equivalent to (AndX X mask), +// and hence the result is always equivalent to (AndX T mask), // The X in AndX must be I or L, depending on bt. // Specifically, this holds for the following cases, // when the shift value N is large enough to zero out From cabc0d85dd81a65e4de430bd11f35eac5a7f35de Mon Sep 17 00:00:00 2001 From: Matthias Ernst Date: Tue, 24 Dec 2024 10:01:18 +0100 Subject: [PATCH 5/7] counter --- .../jtreg/compiler/c2/irTests/TestShiftAndMask.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java b/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java index 098617d29e546..d5766870118d7 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java @@ -150,6 +150,13 @@ public static void addShiftPlusConstMaskInt_runner() { } } + @Test + @Arguments(values = {Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @IR(counts = { IRNode.ADD_I, "2", IRNode.LSHIFT_I, "1" }) + public static int addShiftPlusConstDisjointMaskInt(int i, int j) { + return (j + ((i + 5) << 2)) & 32; // NOT transformed even though (5<<2) & 32 == 0 + } + @Test @IR(counts = { IRNode.AND_I, "1" }) @IR(failOn = { IRNode.ADD_I, IRNode.LSHIFT_I }) From df79a96b36efbd3756874cf63417b3131fc83671 Mon Sep 17 00:00:00 2001 From: Matthias Ernst Date: Tue, 24 Dec 2024 10:14:24 +0100 Subject: [PATCH 6/7] --more null checks --- src/hotspot/share/opto/mulnode.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index a43ba87c512ae..d07833143f02d 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -2121,9 +2121,6 @@ static bool AndIL_is_zero_element(const PhaseGVN* phase, const Node* expr, const Node* MulNode::AndIL_sum_and_mask(PhaseGVN* phase, BasicType bt) { Node* add = in(1); Node* mask = in(2); - if (add == nullptr || mask == nullptr) { - return nullptr; - } int addidx = 0; if (add->Opcode() == Op_Add(bt)) { addidx = 1; @@ -2135,14 +2132,12 @@ Node* MulNode::AndIL_sum_and_mask(PhaseGVN* phase, BasicType bt) { if (addidx > 0) { Node* add1 = add->in(1); Node* add2 = add->in(2); - if (add1 != nullptr && add2 != nullptr) { - if (AndIL_is_zero_element(phase, add1, mask, bt)) { - set_req_X(addidx, add2, phase); - return this; - } else if (AndIL_is_zero_element(phase, add2, mask, bt)) { - set_req_X(addidx, add1, phase); - return this; - } + if (AndIL_is_zero_element(phase, add1, mask, bt)) { + set_req_X(addidx, add2, phase); + return this; + } else if (AndIL_is_zero_element(phase, add2, mask, bt)) { + set_req_X(addidx, add1, phase); + return this; } } return nullptr; From 2a79c24ada6c7b5dfe68aa7f73da258e4baea73f Mon Sep 17 00:00:00 2001 From: Matthias Ernst Date: Tue, 24 Dec 2024 10:19:39 +0100 Subject: [PATCH 7/7] consistently use bt --- src/hotspot/share/opto/mulnode.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index d07833143f02d..e9389960cf655 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -2068,7 +2068,7 @@ static jint AndIL_min_trailing_zeros(const PhaseGVN* phase, const Node* expr, Ba } if (type->is_con()) { - long con = type->get_con_as_long(type->basic_type()); + long con = type->get_con_as_long(bt); return con == 0L ? (type2aelembytes(bt) * BitsPerByte) : count_trailing_zeros(con); } @@ -2078,7 +2078,7 @@ static jint AndIL_min_trailing_zeros(const PhaseGVN* phase, const Node* expr, Ba type = phase->type(expr)->isa_int(); } - if (expr->Opcode() == Op_LShift(type->basic_type())) { + if (expr->Opcode() == Op_LShift(bt)) { const TypeInt* rhs_t = phase->type(expr->in(2))->isa_int(); if (rhs_t == nullptr || !rhs_t->is_con()) { return 0;