8320330: Improve implementation of RShift Value

jaskarth · TobiHartmann · commit 4977922a3f48 · 2023-11-27T14:44:08.000Z
Reviewed-by: thartmann, chagedorn
diff --git 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
@@ -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
@@ -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;
+    }
+}
