From 4977922a3f48613d18da021c619093ce210749f8 Mon Sep 17 00:00:00 2001 From: Jasmine Karthikeyan Date: Mon, 27 Nov 2023 14:44:08 +0000 Subject: [PATCH] 8320330: Improve implementation of RShift Value Reviewed-by: thartmann, chagedorn --- src/hotspot/share/opto/mulnode.cpp | 41 ++++++--- .../irTests/RShiftINodeIdealizationTests.java | 92 +++++++++++++++++++ .../irTests/RShiftLNodeIdealizationTests.java | 92 +++++++++++++++++++ 3 files changed, 214 insertions(+), 11 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/RShiftINodeIdealizationTests.java create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/RShiftLNodeIdealizationTests.java diff --git a/src/hotspot/share/opto/mulnode.cpp b/src/hotspot/share/opto/mulnode.cpp index 6709f831447..3b493b5efa2 100644 --- a/src/hotspot/share/opto/mulnode.cpp +++ b/src/hotspot/share/opto/mulnode.cpp @@ -1297,15 +1297,12 @@ const Type* RShiftINode::Value(PhaseGVN* phase) const { if (t1 == Type::BOTTOM || t2 == Type::BOTTOM) return TypeInt::INT; - if (t2 == TypeInt::INT) - return TypeInt::INT; - const TypeInt *r1 = t1->is_int(); // Handy access const TypeInt *r2 = t2->is_int(); // Handy access // If the shift is a constant, just shift the bounds of the type. // For example, if the shift is 31, we just propagate sign bits. - if (r2->is_con()) { + if (!r1->is_con() && r2->is_con()) { uint shift = r2->get_con(); shift &= BitsPerJavaInteger-1; // semantics of Java shifts // Shift by a multiple of 32 does nothing: @@ -1327,11 +1324,22 @@ const Type* RShiftINode::Value(PhaseGVN* phase) const { return ti; } - if( !r1->is_con() || !r2->is_con() ) + if (!r1->is_con() || !r2->is_con()) { + // If the left input is non-negative the result must also be non-negative, regardless of what the right input is. + if (r1->_lo >= 0) { + return TypeInt::make(0, r1->_hi, MAX2(r1->_widen, r2->_widen)); + } + + // Conversely, if the left input is negative then the result must be negative. + if (r1->_hi <= -1) { + return TypeInt::make(r1->_lo, -1, MAX2(r1->_widen, r2->_widen)); + } + return TypeInt::INT; + } // Signed shift right - return TypeInt::make( r1->get_con() >> (r2->get_con()&31) ); + return TypeInt::make(r1->get_con() >> (r2->get_con() & 31)); } //============================================================================= @@ -1359,15 +1367,12 @@ const Type* RShiftLNode::Value(PhaseGVN* phase) const { if (t1 == Type::BOTTOM || t2 == Type::BOTTOM) return TypeLong::LONG; - if (t2 == TypeInt::INT) - return TypeLong::LONG; - const TypeLong *r1 = t1->is_long(); // Handy access const TypeInt *r2 = t2->is_int (); // Handy access // If the shift is a constant, just shift the bounds of the type. // For example, if the shift is 63, we just propagate sign bits. - if (r2->is_con()) { + if (!r1->is_con() && r2->is_con()) { uint shift = r2->get_con(); shift &= (2*BitsPerJavaInteger)-1; // semantics of Java shifts // Shift by a multiple of 64 does nothing: @@ -1389,7 +1394,21 @@ const Type* RShiftLNode::Value(PhaseGVN* phase) const { return tl; } - return TypeLong::LONG; // Give up + if (!r1->is_con() || !r2->is_con()) { + // If the left input is non-negative the result must also be non-negative, regardless of what the right input is. + if (r1->_lo >= 0) { + return TypeLong::make(0, r1->_hi, MAX2(r1->_widen, r2->_widen)); + } + + // Conversely, if the left input is negative then the result must be negative. + if (r1->_hi <= -1) { + return TypeLong::make(r1->_lo, -1, MAX2(r1->_widen, r2->_widen)); + } + + return TypeLong::LONG; + } + + return TypeLong::make(r1->get_con() >> (r2->get_con() & 63)); } //============================================================================= diff --git a/test/hotspot/jtreg/compiler/c2/irTests/RShiftINodeIdealizationTests.java b/test/hotspot/jtreg/compiler/c2/irTests/RShiftINodeIdealizationTests.java new file mode 100644 index 00000000000..e6501d3728a --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/RShiftINodeIdealizationTests.java @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * 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 jdk.test.lib.Asserts; +import compiler.lib.ir_framework.*; + +/* + * @test + * @bug 8320330 + * @summary Test that RShiftINode optimizations are being performed as expected. + * @library /test/lib / + * @run driver compiler.c2.irTests.RShiftINodeIdealizationTests + */ +public class RShiftINodeIdealizationTests { + public static void main(String[] args) { + TestFramework.run(); + } + + @Run(test = { "test1", "test2", "test3", "test4" }) + public void runMethod() { + int a = RunInfo.getRandom().nextInt(); + int b = RunInfo.getRandom().nextInt(); + int c = RunInfo.getRandom().nextInt(); + int d = RunInfo.getRandom().nextInt(); + + int min = Integer.MIN_VALUE; + int max = Integer.MAX_VALUE; + + assertResult(a, 0); + assertResult(a, b); + assertResult(b, a); + assertResult(c, d); + assertResult(d, c); + assertResult(min, max); + assertResult(max, min); + assertResult(min, min); + assertResult(max, max); + } + + @DontCompile + public void assertResult(int x, int y) { + Asserts.assertEQ((x >> y) >= 0 ? 0 : 1, test1(x, y)); + Asserts.assertEQ(((x & 127) >> y) >= 0 ? 0 : 1, test2(x, y)); + Asserts.assertEQ(((-(x & 127) - 1) >> y) >= 0 ? 0 : 1, test3(x, y)); + Asserts.assertEQ((x >> 30) > 4 ? 0 : 1, test4(x, y)); + } + + @Test + @IR(counts = { IRNode.RSHIFT, "1" }) + public int test1(int x, int y) { + return (x >> y) >= 0 ? 0 : 1; + } + + @Test + @IR(failOn = { IRNode.RSHIFT }) + public int test2(int x, int y) { + return ((x & 127) >> y) >= 0 ? 0 : 1; + } + + @Test + @IR(failOn = { IRNode.RSHIFT }) + public int test3(int x, int y) { + return ((-(x & 127) - 1) >> y) >= 0 ? 0 : 1; + } + + @Test + @IR(failOn = { IRNode.RSHIFT }) + public int test4(int x, int y) { + return (x >> 30) > 4 ? 0 : 1; + } +} diff --git a/test/hotspot/jtreg/compiler/c2/irTests/RShiftLNodeIdealizationTests.java b/test/hotspot/jtreg/compiler/c2/irTests/RShiftLNodeIdealizationTests.java new file mode 100644 index 00000000000..0a8d2bde0b1 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/RShiftLNodeIdealizationTests.java @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * 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 jdk.test.lib.Asserts; +import compiler.lib.ir_framework.*; + +/* + * @test + * @bug 8320330 + * @summary Test that RShiftLNode optimizations are being performed as expected. + * @library /test/lib / + * @run driver compiler.c2.irTests.RShiftLNodeIdealizationTests + */ +public class RShiftLNodeIdealizationTests { + public static void main(String[] args) { + TestFramework.run(); + } + + @Run(test = { "test1", "test2", "test3", "test4" }) + public void runMethod() { + long a = RunInfo.getRandom().nextLong(); + long b = RunInfo.getRandom().nextLong(); + long c = RunInfo.getRandom().nextLong(); + long d = RunInfo.getRandom().nextLong(); + + long min = Long.MIN_VALUE; + long max = Long.MAX_VALUE; + + assertResult(a, 0); + assertResult(a, b); + assertResult(b, a); + assertResult(c, d); + assertResult(d, c); + assertResult(min, max); + assertResult(max, min); + assertResult(min, min); + assertResult(max, max); + } + + @DontCompile + public void assertResult(long x, long y) { + Asserts.assertEQ((x >> y) >= 0 ? 0L : 1L, test1(x, y)); + Asserts.assertEQ(((x & 127) >> y) >= 0 ? 0L : 1L, test2(x, y)); + Asserts.assertEQ(((-(x & 127) - 1) >> y) >= 0 ? 0L : 1L, test3(x, y)); + Asserts.assertEQ((x >> 62) > 4 ? 0L : 1L, test4(x, y)); + } + + @Test + @IR(counts = { IRNode.RSHIFT, "1" }) + public long test1(long x, long y) { + return (x >> y) >= 0 ? 0 : 1; + } + + @Test + @IR(failOn = { IRNode.RSHIFT }) + public long test2(long x, long y) { + return ((x & 127) >> y) >= 0 ? 0L : 1L; + } + + @Test + @IR(failOn = { IRNode.RSHIFT }) + public long test3(long x, long y) { + return ((-(x & 127) - 1) >> y) >= 0 ? 0L : 1L; + } + + @Test + @IR(failOn = { IRNode.RSHIFT }) + public long test4(long x, long y) { + return (x >> 62) > 4 ? 0L : 1L; + } +}