8294816: C2: Math.min/max vectorization miscompilation

Reviewed-by: thartmann, ngasson

Author: Bhavana Kilambi

src/hotspot/share/opto/superword.cpp

@@ -2093,6 +2093,12 @@ bool SuperWord::implemented(Node_List* p) {
       }
     } else if (VectorNode::is_convert_opcode(opc)) {
       retValue = VectorCastNode::implemented(opc, size, velt_basic_type(p0->in(1)), velt_basic_type(p0));
+    } else if (VectorNode::is_minmax_opcode(opc) && is_subword_type(velt_basic_type(p0))) {
+      // Java API for Math.min/max operations supports only int, long, float
+      // and double types. Thus, avoid generating vector min/max nodes for
+      // integer subword types with superword vectorization.
+      // See JDK-8294816 for miscompilation issues with shorts.
+      return false;
     } else {
       // Vector unsigned right shift for signed subword types behaves differently
       // from Java Spec. But when the shift amount is a constant not greater than

src/hotspot/share/opto/vectornode.cpp

@@ -476,6 +476,10 @@ bool VectorNode::is_convert_opcode(int opc) {
   }
 }
 
+bool VectorNode::is_minmax_opcode(int opc) {
+  return opc == Op_MinI || opc == Op_MaxI;
+}
+
 bool VectorNode::is_shift(Node* n) {
   return is_shift_opcode(n->Opcode());
 }

src/hotspot/share/opto/vectornode.hpp

@@ -84,6 +84,7 @@ class VectorNode : public TypeNode {
   static bool is_shift_opcode(int opc);
   static bool can_transform_shift_op(Node* n, BasicType bt);
   static bool is_convert_opcode(int opc);
+  static bool is_minmax_opcode(int opc);
 
   static bool is_vshift_cnt_opcode(int opc);
 

test/hotspot/jtreg/compiler/c2/TestMinMaxSubword.java

@@ -0,0 +1,128 @@
+/*
+ * Copyright (c) 2022, Arm Limited. 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;
+
+import compiler.lib.ir_framework.*;
+import jdk.test.lib.Asserts;
+import jdk.test.lib.Utils;
+import java.util.Random;
+
+/*
+ * @test
+ * @bug 8294816
+ * @summary Test Math.min/max vectorization miscompilation for integer subwords
+ * @library /test/lib /
+ * @requires vm.compiler2.enabled
+ * @run driver compiler.c2.TestMinMaxSubword
+ */
+
+public class TestMinMaxSubword {
+    private static final int LENGTH = 256;
+    private static final Random RANDOM = Utils.getRandomInstance();
+    private static int val = 65536;
+    private static short[] sa;
+    private static short[] sb;
+    private static byte[] ba;
+    private static byte[] bb;
+
+    static {
+        sa = new short[LENGTH];
+        sb = new short[LENGTH];
+        ba = new byte[LENGTH];
+        bb = new byte[LENGTH];
+        for(int i = 0; i < LENGTH; i++) {
+            sa[i] = (short) (RANDOM.nextInt(999) + 1);
+            ba[i] = (byte) (RANDOM.nextInt(99) + 1);
+        }
+    }
+
+    // Ensure vector max/min instructions are not generated for integer subword types
+    // as Java APIs for Math.min/max do not support integer subword types and superword
+    // should not generate vectorized Min/Max nodes for them.
+    @Test
+    @IR(failOn = {IRNode.MIN_V})
+    public static void testMinShort() {
+        for (int i = 0; i < LENGTH; i++) {
+           sb[i] = (short) Math.min(sa[i], val);
+        }
+    }
+
+    @Run(test = "testMinShort")
+    public static void testMinShort_runner() {
+        testMinShort();
+        for (int i = 0; i < LENGTH; i++) {
+            Asserts.assertEquals(sb[i], sa[i]);
+        }
+    }
+
+    @Test
+    @IR(failOn = {IRNode.MAX_V})
+    public static void testMaxShort() {
+        for (int i = 0; i < LENGTH; i++) {
+            sb[i] = (short) Math.max(sa[i], val);
+        }
+    }
+    @Run(test = "testMaxShort")
+    public static void testMaxShort_runner() {
+        testMaxShort();
+        for (int i = 0; i < LENGTH; i++) {
+            Asserts.assertEquals(sb[i], (short) 0);
+        }
+    }
+
+    @Test
+    @IR(failOn = {IRNode.MIN_V})
+    public static void testMinByte() {
+        for (int i = 0; i < LENGTH; i++) {
+           bb[i] = (byte) Math.min(ba[i], val);
+        }
+    }
+
+    @Run(test = "testMinByte")
+    public static void testMinByte_runner() {
+        testMinByte();
+        for (int i = 0; i < LENGTH; i++) {
+            Asserts.assertEquals(bb[i], ba[i]);
+        }
+    }
+
+    @Test
+    @IR(failOn = {IRNode.MAX_V})
+    public static void testMaxByte() {
+        for (int i = 0; i < LENGTH; i++) {
+            bb[i] = (byte) Math.max(ba[i], val);
+        }
+    }
+    @Run(test = "testMaxByte")
+    public static void testMaxByte_runner() {
+        testMaxByte();
+        for (int i = 0; i < LENGTH; i++) {
+            Asserts.assertEquals(bb[i], (byte) 0);
+        }
+    }
+
+    public static void main(String[] args) {
+        TestFramework.run();
+    }
+}