From 4dacb0521abf474904cd0c028e9ca870d6261ff6 Mon Sep 17 00:00:00 2001 From: rwestrel Date: Thu, 25 Nov 2021 17:35:53 +0100 Subject: [PATCH 1/5] fix --- src/hotspot/share/opto/mulnode.cpp | 87 +++++++++++++- src/hotspot/share/opto/mulnode.hpp | 5 + .../compiler/c2/irTests/TestShiftAndMask.java | 108 ++++++++++++++++++ .../compiler/lib/ir_framework/IRNode.java | 8 ++ 4 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index a45ad23f6c159..717ba5cd55663 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -505,6 +505,15 @@ const Type *AndINode::mul_ring( const Type *t0, const Type *t1 ) const { return TypeInt::INT; // No constants to be had } +const Type* AndINode::Value(PhaseGVN* phase) const { + // patterns similar to (v << 2) & 3 + if (AndIL_shift_and_mask(phase, in(2), in(1), T_INT)) { + return TypeInt::ZERO; + } + + return MulINode::Value(phase); +} + //------------------------------Identity--------------------------------------- // Masking off the high bits of an unsigned load is not required Node* AndINode::Identity(PhaseGVN* phase) { @@ -598,6 +607,12 @@ Node *AndINode::Ideal(PhaseGVN *phase, bool can_reshape) { phase->type(load->in(1)) == TypeInt::ZERO ) return new AndINode( load->in(2), in(2) ); + // pattern similar to (v1 + (v2 << 2)) & 3 transformed to v1 & 3 + Node* progress = AndIL_add_shift_and_mask(phase, T_INT); + if (progress != NULL) { + return progress; + } + return MulNode::Ideal(phase, can_reshape); } @@ -629,6 +644,15 @@ const Type *AndLNode::mul_ring( const Type *t0, const Type *t1 ) const { return TypeLong::LONG; // No constants to be had } +const Type* AndLNode::Value(PhaseGVN* phase) const { + // patterns similar to (v << 2) & 3 + if (AndIL_shift_and_mask(phase, in(2), in(1), T_LONG)) { + return TypeLong::ZERO; + } + + return MulLNode::Value(phase); +} + //------------------------------Identity--------------------------------------- // Masking off the high bits of an unsigned load is not required Node* AndLNode::Identity(PhaseGVN* phase) { @@ -675,7 +699,7 @@ Node *AndLNode::Ideal(PhaseGVN *phase, bool can_reshape) { const jlong mask = t2->get_con(); Node* in1 = in(1); - uint op = in1->Opcode(); + int op = in1->Opcode(); // Are we masking a long that was converted from an int with a mask // that fits in 32-bits? Commute them and use an AndINode. Don't @@ -705,6 +729,12 @@ 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); + if (progress != NULL) { + return progress; + } + return MulNode::Ideal(phase, can_reshape); } @@ -1683,3 +1713,58 @@ const Type* RotateRightNode::Value(PhaseGVN* phase) const { return TypeLong::LONG; } } + +bool MulNode::AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, BasicType bt) const { + if (mask == NULL || shift == NULL) { + return false; + } + const TypeInteger* mask_t = phase->type(mask)->isa_integer(bt); + const TypeInteger* shift_t = phase->type(shift)->isa_integer(bt); + if (mask_t == NULL || shift_t == NULL) { + return false; + } + if (bt == T_LONG && shift != NULL && shift->Opcode() == Op_ConvI2L) { + bt = T_INT; + shift = shift->in(1); + if (shift == NULL) { + return false; + } + } + if (shift->Opcode() != Op_LShift(bt)) { + return false; + } + Node* shift2 = shift->in(2); + if (shift2 == NULL) { + 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() & ((bt == T_INT ? BitsPerJavaInteger : BitsPerJavaLong) - 1); + if ((1L << shift_con) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0) { + return true; + } + + return false; +} + +Node* MulNode::AndIL_add_shift_and_mask(PhaseGVN* phase, BasicType bt) { + Node* in1 = in(1); + Node* in2 = in(2); + if (in1 != NULL && in2 != NULL && in1->Opcode() == Op_Add(bt)) { + Node* add1 = in1->in(1); + Node* add2 = in1->in(2); + if (add1 != NULL && add2 != NULL) { + if (AndIL_shift_and_mask(phase, in2, add1, bt)) { + set_req_X(1, add2, phase); + return this; + } else if (AndIL_shift_and_mask(phase, in2, add2, bt)) { + set_req_X(1, add1, phase); + return this; + } + } + } + return NULL; +} diff --git a/src/hotspot/share/opto/mulnode.hpp b/src/hotspot/share/opto/mulnode.hpp index 3c7f27910a7f6..698f56db19983 100644 --- a/src/hotspot/share/opto/mulnode.hpp +++ b/src/hotspot/share/opto/mulnode.hpp @@ -82,6 +82,9 @@ class MulNode : public Node { virtual int min_opcode() const = 0; static MulNode* make(Node* in1, Node* in2, BasicType bt); + + bool AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, BasicType bt) const; + Node* AndIL_add_shift_and_mask(PhaseGVN* phase, BasicType bt); }; //------------------------------MulINode--------------------------------------- @@ -189,6 +192,7 @@ class AndINode : public MulINode { virtual int Opcode() const; virtual Node *Ideal(PhaseGVN *phase, bool can_reshape); virtual Node* Identity(PhaseGVN* phase); + virtual const Type* Value(PhaseGVN* phase) const; virtual const Type *mul_ring( const Type *, const Type * ) const; const Type *mul_id() const { return TypeInt::MINUS_1; } const Type *add_id() const { return TypeInt::ZERO; } @@ -208,6 +212,7 @@ class AndLNode : public MulLNode { virtual int Opcode() const; virtual Node *Ideal(PhaseGVN *phase, bool can_reshape); virtual Node* Identity(PhaseGVN* phase); + virtual const Type* Value(PhaseGVN* phase) const; virtual const Type *mul_ring( const Type *, const Type * ) const; const Type *mul_id() const { return TypeLong::MINUS_1; } const Type *add_id() const { return TypeLong::ZERO; } diff --git a/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java b/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java new file mode 100644 index 0000000000000..ec20fcf97e84d --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java @@ -0,0 +1,108 @@ +/* + * Copyright (c) 2021, Red Hat, Inc. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package compiler.c2.irTests; + +import compiler.lib.ir_framework.*; + +/* + * @test + * @bug JDK-8277850 + * @summary C2: optimize mask checks in counted loops + * @library /test/lib / + * @run driver compiler.c2.irTests.TestShiftAndMask + */ + +public class TestShiftAndMask { + public static void main(String[] args) { + TestFramework.run(); + } + + @Test + @Arguments(Argument.RANDOM_EACH) + @IR(failOn = { IRNode.AND_I, IRNode.LSHIFT_I }) + public static int shiftMaskInt(int i) { + return (i << 2) & 3; // transformed to: return 0; + } + + @Test + @Arguments(Argument.RANDOM_EACH) + @IR(failOn = { IRNode.AND_L, IRNode.LSHIFT_L }) + public static long shiftMaskLong(long i) { + return (i << 2) & 3; // transformed to: return 0; + } + + @Test + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @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; + } + + @Test + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @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; + } + + @Test + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @IR(failOn = { IRNode.AND_I, IRNode.ADD_I, IRNode.LSHIFT_I }) + public static int addShiftMaskInt2(int i, int j) { + return ((j << 2) + (i << 2)) & 3; // transformed to: return 0; + } + + @Test + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @IR(failOn = { IRNode.AND_L, IRNode.ADD_L, IRNode.LSHIFT_L }) + public static long addShiftMaskLong2(long i, long j) { + return ((j << 2) + (i << 2)) & 3; // transformed to: return 0; + } + + @Test + @Arguments(Argument.RANDOM_EACH) + @IR(failOn = { IRNode.AND_L, IRNode.LSHIFT_I }) + public static long shiftConvMask(int i) { + return ((long)(i << 2)) & 3; // transformed to: return 0; + } + + @Test + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @IR(counts = { IRNode.AND_L, "1" }) + @IR(failOn = { IRNode.ADD_L, IRNode.LSHIFT_I, IRNode.CONV_I2L }) + public static long addShiftConvMask(int i, long j) { + return (j + (i << 2)) & 3; // transformed to: return j & 3; + } + + @Test + @Arguments({Argument.RANDOM_EACH, Argument.RANDOM_EACH}) + @IR(failOn = { IRNode.AND_L, IRNode.ADD_L, IRNode.LSHIFT_L }) + public static long addShiftConvMask2(int i, int j) { + return (((long)(j << 2)) + ((long)(i << 2))) & 3; // transformed to: return 0; + } + +} + diff --git a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java index a995cad3f2d49..18875cd937762 100644 --- a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java +++ b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java @@ -133,6 +133,14 @@ public class IRNode { public static final String SCOPE_OBJECT = "(.*# ScObj.*" + END; public static final String MEMBAR = START + "MemBar" + MID + END; + public static final String AND_I = START + "AndI" + MID + END; + public static final String AND_L = START + "AndL" + MID + END; + public static final String LSHIFT_I = START + "LShiftI" + MID + END; + public static final String LSHIFT_L = START + "LShiftL" + MID + END; + public static final String ADD_I = START + "AddI" + MID + END; + public static final String ADD_L = START + "AddL" + MID + END; + public static final String CONV_I2L = START + "ConvI2L" + MID + END; + /** * Called by {@link IRMatcher} to merge special composite nodes together with additional user-defined input. */ From ab48aac9095224301401bbc16a29415ca2bf7103 Mon Sep 17 00:00:00 2001 From: rwestrel Date: Fri, 3 Dec 2021 11:14:10 +0100 Subject: [PATCH 2/5] whitespace --- src/hotspot/share/opto/mulnode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index 717ba5cd55663..74800d8059b7d 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -1746,7 +1746,7 @@ bool MulNode::AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, Bas if ((1L << shift_con) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0) { return true; } - + return false; } From ea0464b27326e73f8cca89358bf3f2b332b0b8f5 Mon Sep 17 00:00:00 2001 From: rwestrel Date: Tue, 7 Dec 2021 11:49:42 +0100 Subject: [PATCH 3/5] reviews --- src/hotspot/share/opto/mulnode.cpp | 12 +++++++++--- src/hotspot/share/opto/mulnode.hpp | 2 +- .../jtreg/compiler/c2/irTests/TestShiftAndMask.java | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index 74800d8059b7d..5c722cc018439 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -511,7 +511,7 @@ const Type* AndINode::Value(PhaseGVN* phase) const { return TypeInt::ZERO; } - return MulINode::Value(phase); + return MulNode::Value(phase); } //------------------------------Identity--------------------------------------- @@ -650,7 +650,7 @@ const Type* AndLNode::Value(PhaseGVN* phase) const { return TypeLong::ZERO; } - return MulLNode::Value(phase); + return MulNode::Value(phase); } //------------------------------Identity--------------------------------------- @@ -1714,7 +1714,11 @@ const Type* RotateRightNode::Value(PhaseGVN* phase) const { } } -bool MulNode::AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, BasicType bt) const { +// Helper method to transform: +// patterns similar to (v << 2) & 3 to 0 +// and +// patterns similar to (v1 + (v2 << 2)) & 3 transformed to v1 & 3 +bool MulNode::AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, BasicType bt) { if (mask == NULL || shift == NULL) { return false; } @@ -1750,6 +1754,8 @@ bool MulNode::AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, Bas return false; } +// Helper method to transform: +// patterns similar to (v1 + (v2 << 2)) & 3 to v1 & 3 Node* MulNode::AndIL_add_shift_and_mask(PhaseGVN* phase, BasicType bt) { Node* in1 = in(1); Node* in2 = in(2); diff --git a/src/hotspot/share/opto/mulnode.hpp b/src/hotspot/share/opto/mulnode.hpp index 698f56db19983..ec751aec79922 100644 --- a/src/hotspot/share/opto/mulnode.hpp +++ b/src/hotspot/share/opto/mulnode.hpp @@ -83,7 +83,7 @@ class MulNode : public Node { static MulNode* make(Node* in1, Node* in2, BasicType bt); - bool AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, BasicType bt) const; + static bool AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, BasicType bt); Node* AndIL_add_shift_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 ec20fcf97e84d..ca6d59428a88a 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/TestShiftAndMask.java @@ -27,7 +27,7 @@ /* * @test - * @bug JDK-8277850 + * @bug 8277850 * @summary C2: optimize mask checks in counted loops * @library /test/lib / * @run driver compiler.c2.irTests.TestShiftAndMask From 78b10f4dea4bcc2e355a33ab4cdd14e2a82724fe Mon Sep 17 00:00:00 2001 From: rwestrel Date: Wed, 8 Dec 2021 09:09:23 +0100 Subject: [PATCH 4/5] build fix --- src/hotspot/share/opto/mulnode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index 5c722cc018439..6afa7eeffa316 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -1747,7 +1747,7 @@ bool MulNode::AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, Bas } jint shift_con = shift2_t->is_int()->get_con() & ((bt == T_INT ? BitsPerJavaInteger : BitsPerJavaLong) - 1); - if ((1L << shift_con) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0) { + if ((((long)1) << shift_con) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0) { return true; } From 94b6869354c7164c05fc6b75ace9f2df31f739ed Mon Sep 17 00:00:00 2001 From: rwestrel Date: Wed, 8 Dec 2021 10:43:33 +0100 Subject: [PATCH 5/5] build fix --- src/hotspot/share/opto/mulnode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index 6afa7eeffa316..e28ccb6e94a5f 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -1747,7 +1747,7 @@ bool MulNode::AndIL_shift_and_mask(PhaseGVN* phase, Node* mask, Node* shift, Bas } jint shift_con = shift2_t->is_int()->get_con() & ((bt == T_INT ? BitsPerJavaInteger : BitsPerJavaLong) - 1); - if ((((long)1) << shift_con) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0) { + if ((((jlong)1) << shift_con) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0) { return true; }