8152179: C2: Folding of unsigned loads is broken w/ @Stable

Reviewed-by: jrose, kvn

hotspot/src/share/vm/opto/memnode.cpp

@@ -1620,7 +1620,7 @@ LoadNode::load_array_final_field(const TypeKlassPtr *tkls,
   return NULL;
 }
 
-static bool is_mismatched_access(ciConstant con, BasicType loadbt) {
+static ciConstant check_mismatched_access(ciConstant con, BasicType loadbt, bool is_unsigned) {
   BasicType conbt = con.basic_type();
   switch (conbt) {
     case T_BOOLEAN: conbt = T_BYTE;   break;
@@ -1632,23 +1632,40 @@ static bool is_mismatched_access(ciConstant con, BasicType loadbt) {
     case T_ARRAY:     loadbt = T_OBJECT; break;
     case T_ADDRESS:   loadbt = T_OBJECT; break;
   }
-  return (conbt != loadbt);
+  if (conbt == loadbt) {
+    if (is_unsigned && conbt == T_BYTE) {
+      // LoadB (T_BYTE) with a small mask (<=8-bit) is converted to LoadUB (T_BYTE).
+      return ciConstant(T_INT, con.as_int() & 0xFF);
+    } else {
+      return con;
+    }
+  }
+  if (conbt == T_SHORT && loadbt == T_CHAR) {
+    // LoadS (T_SHORT) with a small mask (<=16-bit) is converted to LoadUS (T_CHAR).
+    return ciConstant(T_INT, con.as_int() & 0xFFFF);
+  }
+  return ciConstant(); // T_ILLEGAL
 }
 
 // Try to constant-fold a stable array element.
-static const Type* fold_stable_ary_elem(const TypeAryPtr* ary, int off, BasicType loadbt) {
+static const Type* fold_stable_ary_elem(const TypeAryPtr* ary, int off, bool is_unsigned_load, BasicType loadbt) {
   assert(ary->const_oop(), "array should be constant");
   assert(ary->is_stable(), "array should be stable");
 
   // Decode the results of GraphKit::array_element_address.
   ciArray* aobj = ary->const_oop()->as_array();
-  ciConstant con = aobj->element_value_by_offset(off);
-  if (con.basic_type() != T_ILLEGAL && !con.is_null_or_zero()) {
-    bool is_mismatched = is_mismatched_access(con, loadbt);
-    assert(!is_mismatched, "conbt=%s; loadbt=%s", type2name(con.basic_type()), type2name(loadbt));
+  ciConstant element_value = aobj->element_value_by_offset(off);
+  if (element_value.basic_type() == T_ILLEGAL) {
+    return NULL; // wrong offset
+  }
+  ciConstant con = check_mismatched_access(element_value, loadbt, is_unsigned_load);
+  assert(con.basic_type() != T_ILLEGAL, "elembt=%s; loadbt=%s; unsigned=%d",
+         type2name(element_value.basic_type()), type2name(loadbt), is_unsigned_load);
+
+  if (con.basic_type() != T_ILLEGAL && // not a mismatched access
+      !con.is_null_or_zero()) {        // not a default value
     const Type* con_type = Type::make_from_constant(con);
-    // Guard against erroneous constant folding.
-    if (!is_mismatched && con_type != NULL) {
+    if (con_type != NULL) {
       if (con_type->isa_aryptr()) {
         // Join with the array element type, in case it is also stable.
         int dim = ary->stable_dimension();
@@ -1700,7 +1717,7 @@ const Type* LoadNode::Value(PhaseGVN* phase) const {
     if (FoldStableValues && !is_mismatched_access() && ary->is_stable() && ary->const_oop() != NULL) {
       // Make sure the reference is not into the header and the offset is constant
       if (off_beyond_header && adr->is_AddP() && off != Type::OffsetBot) {
-        const Type* con_type = fold_stable_ary_elem(ary, off, memory_type());
+        const Type* con_type = fold_stable_ary_elem(ary, off, is_unsigned(), memory_type());
         if (con_type != NULL) {
           return con_type;
         }

hotspot/src/share/vm/opto/memnode.hpp

@@ -197,6 +197,10 @@ public:
     assert(_mo == unordered || _mo == acquire, "unexpected");
     return _mo == acquire;
   }
+  inline bool is_unsigned() const {
+    int lop = Opcode();
+    return (lop == Op_LoadUB) || (lop == Op_LoadUS);
+  }
 
   // Polymorphic factory method:
   static Node* make(PhaseGVN& gvn, Node *c, Node *mem, Node *adr,

hotspot/test/compiler/stable/TestStableUByte.java

@@ -0,0 +1,155 @@
+/*
+ * Copyright (c) 2016, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+/*
+ * @test TestStableUByte
+ * @summary tests on stable fields and arrays
+ * @library /testlibrary /test/lib
+ * @build TestStableUByte StableConfiguration sun.hotspot.WhiteBox
+ * @run main ClassFileInstaller sun.hotspot.WhiteBox sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @run main ClassFileInstaller
+ *           java/lang/invoke/StableConfiguration
+ *           java/lang/invoke/TestStableUByte
+ *           java/lang/invoke/TestStableUByte$UByteStable
+ *           java/lang/invoke/TestStableUByte$UByteArrayDim1
+ *
+ * @run main/othervm -Xbootclasspath/a:.
+ *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xcomp
+ *                   -XX:-TieredCompilation
+ *                   -XX:+FoldStableValues
+ *                   -XX:CompileOnly=::get,::get1
+ *                   java.lang.invoke.TestStableUByte
+ * @run main/othervm -Xbootclasspath/a:.
+ *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xcomp
+ *                   -XX:-TieredCompilation
+ *                   -XX:-FoldStableValues
+ *                   -XX:CompileOnly=::get,::get1
+ *                   java.lang.invoke.TestStableUByte
+ *
+ * @run main/othervm -Xbootclasspath/a:.
+ *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xcomp
+ *                   -XX:+TieredCompilation -XX:TieredStopAtLevel=1
+ *                   -XX:+FoldStableValues
+ *                   -XX:CompileOnly=::get,::get1
+ *                   java.lang.invoke.TestStableUByte
+ * @run main/othervm -Xbootclasspath/a:.
+ *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xcomp
+ *                   -XX:+TieredCompilation -XX:TieredStopAtLevel=1
+ *                   -XX:-FoldStableValues
+ *                   -XX:CompileOnly=::get,::get1
+ *                   java.lang.invoke.TestStableUByte
+ *
+ */
+package java.lang.invoke;
+
+import jdk.internal.vm.annotation.Stable;
+
+import java.lang.reflect.InvocationTargetException;
+
+public class TestStableUByte {
+    static final boolean isStableEnabled = StableConfiguration.isStableEnabled;
+
+    public static void main(String[] args) throws Exception {
+        run(UByteStable.class);
+        run(UByteArrayDim1.class);
+
+        if (failed) {
+            throw new Error("TEST FAILED");
+        }
+    }
+
+    /* ==================================================== */
+
+    static class UByteStable {
+        public @Stable byte v;
+
+        public static final UByteStable c = new UByteStable();
+
+        public static int get() { return c.v & 0xFF; }
+
+        public static void test() throws Exception {
+            byte v1 = -1, v2 = 1;
+
+            c.v = v1; int r1 = get();
+            c.v = v2; int r2 = get();
+
+            assertEquals(r1, v1 & 0xFF);
+            assertEquals(r2, (isStableEnabled ? v1 : v2) & 0xFF);
+        }
+    }
+
+    /* ==================================================== */
+
+    static class UByteArrayDim1 {
+        public @Stable byte[] v;
+
+        public static final UByteArrayDim1 c = new UByteArrayDim1();
+
+        public static byte[] get()  { return c.v; }
+        public static int    get1() { return get()[0] & 0xFF; }
+
+        public static void test() throws Exception {
+            byte v1 = -1, v2 = 1;
+
+            c.v = new byte[1];
+            c.v[0] = v1; int r1 = get1();
+            c.v[0] = v2; int r2 = get1();
+
+            assertEquals(r1, v1 & 0xFF);
+            assertEquals(r2, (isStableEnabled ? v1 : v2) & 0xFF);
+        }
+    }
+
+    /* ==================================================== */
+    // Auxiliary methods
+    static void assertEquals(int i, int j) { if (i != j)  throw new AssertionError(i + " != " + j); }
+    static void assertTrue(boolean b) { if (!b)  throw new AssertionError(); }
+
+    static boolean failed = false;
+
+    public static void run(Class<?> test) {
+        Throwable ex = null;
+        System.out.print(test.getName()+": ");
+        try {
+            test.getMethod("test").invoke(null);
+        } catch (InvocationTargetException e) {
+            ex = e.getCause();
+        } catch (Throwable e) {
+            ex = e;
+        } finally {
+            if (ex == null) {
+                System.out.println("PASSED");
+            } else {
+                failed = true;
+                System.out.println("FAILED");
+                ex.printStackTrace(System.out);
+            }
+        }
+    }
+}

hotspot/test/compiler/stable/TestStableUShort.java

@@ -0,0 +1,155 @@
+/*
+ * Copyright (c) 2016, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+/*
+ * @test TestStableUShort
+ * @summary tests on stable fields and arrays
+ * @library /testlibrary /test/lib
+ * @build TestStableUShort StableConfiguration sun.hotspot.WhiteBox
+ * @run main ClassFileInstaller sun.hotspot.WhiteBox sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @run main ClassFileInstaller
+ *           java/lang/invoke/StableConfiguration
+ *           java/lang/invoke/TestStableUShort
+ *           java/lang/invoke/TestStableUShort$UShortStable
+ *           java/lang/invoke/TestStableUShort$UShortArrayDim1
+ *
+ * @run main/othervm -Xbootclasspath/a:.
+ *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xcomp
+ *                   -XX:-TieredCompilation
+ *                   -XX:+FoldStableValues
+ *                   -XX:CompileOnly=::get,::get1
+ *                   java.lang.invoke.TestStableUShort
+ * @run main/othervm -Xbootclasspath/a:.
+ *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xcomp
+ *                   -XX:-TieredCompilation
+ *                   -XX:-FoldStableValues
+ *                   -XX:CompileOnly=::get,::get1
+ *                   java.lang.invoke.TestStableUShort
+ *
+ * @run main/othervm -Xbootclasspath/a:.
+ *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xcomp
+ *                   -XX:+TieredCompilation -XX:TieredStopAtLevel=1
+ *                   -XX:+FoldStableValues
+ *                   -XX:CompileOnly=::get,::get1
+ *                   java.lang.invoke.TestStableUShort
+ * @run main/othervm -Xbootclasspath/a:.
+ *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xcomp
+ *                   -XX:+TieredCompilation -XX:TieredStopAtLevel=1
+ *                   -XX:-FoldStableValues
+ *                   -XX:CompileOnly=::get,::get1
+ *                   java.lang.invoke.TestStableUShort
+ *
+ */
+package java.lang.invoke;
+
+import jdk.internal.vm.annotation.Stable;
+
+import java.lang.reflect.InvocationTargetException;
+
+public class TestStableUShort {
+    static final boolean isStableEnabled = StableConfiguration.isStableEnabled;
+
+    public static void main(String[] args) throws Exception {
+        run(UShortStable.class);
+        run(UShortArrayDim1.class);
+
+        if (failed) {
+            throw new Error("TEST FAILED");
+        }
+    }
+
+    /* ==================================================== */
+
+    static class UShortStable {
+        public @Stable short v;
+
+        public static final UShortStable c = new UShortStable();
+
+        public static int get() { return c.v & 0xFFFF; }
+
+        public static void test() throws Exception {
+            short v1 = -1, v2 = 1;
+
+            c.v = v1; int r1 = get();
+            c.v = v2; int r2 = get();
+
+            assertEquals(r1, v1 & 0xFFFF);
+            assertEquals(r2, (isStableEnabled ? v1 : v2) & 0xFFFF);
+        }
+    }
+
+    /* ==================================================== */
+
+    static class UShortArrayDim1 {
+        public @Stable short[] v;
+
+        public static final UShortArrayDim1 c = new UShortArrayDim1();
+
+        public static short[] get()  { return c.v; }
+        public static int    get1() { return get()[0] & 0xFFFF; }
+
+        public static void test() throws Exception {
+            short v1 = -1, v2 = 1;
+
+            c.v = new short[1];
+            c.v[0] = v1; int r1 = get1();
+            c.v[0] = v2; int r2 = get1();
+
+            assertEquals(r1, v1 & 0xFFFF);
+            assertEquals(r2, (isStableEnabled ? v1 : v2) & 0xFFFF);
+        }
+    }
+
+    /* ==================================================== */
+    // Auxiliary methods
+    static void assertEquals(int i, int j) { if (i != j)  throw new AssertionError(i + " != " + j); }
+    static void assertTrue(boolean b) { if (!b)  throw new AssertionError(); }
+
+    static boolean failed = false;
+
+    public static void run(Class<?> test) {
+        Throwable ex = null;
+        System.out.print(test.getName()+": ");
+        try {
+            test.getMethod("test").invoke(null);
+        } catch (InvocationTargetException e) {
+            ex = e.getCause();
+        } catch (Throwable e) {
+            ex = e;
+        } finally {
+            if (ex == null) {
+                System.out.println("PASSED");
+            } else {
+                failed = true;
+                System.out.println("FAILED");
+                ex.printStackTrace(System.out);
+            }
+        }
+    }
+}