changeset 51265:54f37cb89cb3 lworld

8207332: [Lworld] Javac generates bad code for value constructors involving chained assignments
author sadayapalam
date Mon, 16 Jul 2018 22:09:28 +0530
parents 9e35bb8ec76c
children b0224b61fd22
files src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java test/langtools/tools/javac/valhalla/lworld-values/ChainedAssignmentTest.java test/langtools/tools/javac/valhalla/lworld-values/ValueCreationTest.java test/langtools/tools/javac/valhalla/lworld-values/WithFieldOfExplicitSelector.java
diffstat 4 files changed, 88 insertions(+), 20 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java	Mon Jul 16 20:28:32 2018 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java	Mon Jul 16 22:09:28 2018 +0530
@@ -1145,8 +1145,11 @@
             case SELECT:
                 JCFieldAccess fieldAccess = (JCFieldAccess) tree.field;
                 sym = TreeInfo.symbol(fieldAccess);
+                // JDK-8207332: To maintain the order of side effects, must compute value ahead of field
+                genExpr(tree.value, tree.field.type).load();
                 genExpr(fieldAccess.selected, fieldAccess.selected.type).load();
-                genExpr(tree.value, tree.field.type).load();
+                code.emitop0(Code.width(tree.field.type) == 2 ?  dup_x2 : dup_x1);
+                code.emitop0(pop);
                 sym = binaryQualifier(sym, fieldAccess.selected.type);
                 code.emitop2(withfield, pool.put(sym));
                 result = items.makeStackItem(tree.type);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/valhalla/lworld-values/ChainedAssignmentTest.java	Mon Jul 16 22:09:28 2018 +0530
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2018, 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.
+ */
+
+/**
+ * @test
+ * @bug 8207332
+ * @summary Verify that chained assignments in value constructors are lowered correctly.
+ * @run main/othervm -XX:+EnableValhalla ChainedAssignmentTest
+ */
+
+public class ChainedAssignmentTest {
+
+	static __ByValue class Point {
+		int x;
+		int y;
+		Point() {
+			x = y = 1234; // Problematic
+		}
+	}
+
+	static __ByValue class LongPoint {
+		long x;
+		long y;
+		LongPoint() {
+			x = y = 1234; // Problematic
+		}
+	}
+	public static void main(String[] args) {
+		Point p = new Point();
+        if (!p.toString().equals("[value class ChainedAssignmentTest$Point, 1234, 1234]"))
+            throw new AssertionError("Broken");
+
+        LongPoint lp = new LongPoint();
+        if (!lp.toString().equals("[value class ChainedAssignmentTest$LongPoint, 1234, 1234]"))
+            throw new AssertionError("Broken");
+	}
+}
\ No newline at end of file
--- a/test/langtools/tools/javac/valhalla/lworld-values/ValueCreationTest.java	Mon Jul 16 20:28:32 2018 +0530
+++ b/test/langtools/tools/javac/valhalla/lworld-values/ValueCreationTest.java	Mon Jul 16 22:09:28 2018 +0530
@@ -70,14 +70,18 @@
 
          "0: defaultvalue  #2                  // class ValueCreationTest$Point",
          "3: astore_2",
-         "4: aload_2",
-         "5: iload_0",
-         "6: withfield     #3                  // Field x:I",
-         "9: astore_2",
-        "10: aload_2",
-        "11: iload_1",
-        "12: withfield     #4                  // Field y:I",
-        "15: areturn"
+         "4: iload_0",
+         "5: aload_2",
+         "6: dup_x1",
+         "7: pop",
+         "8: withfield     #3                  // Field x:I",
+        "11: astore_2",
+        "12: iload_1",
+        "13: aload_2",
+        "14: dup_x1",
+        "15: pop",
+        "16: withfield     #4                  // Field y:I",
+        "19: areturn"
            
          });
 
--- a/test/langtools/tools/javac/valhalla/lworld-values/WithFieldOfExplicitSelector.java	Mon Jul 16 20:28:32 2018 +0530
+++ b/test/langtools/tools/javac/valhalla/lworld-values/WithFieldOfExplicitSelector.java	Mon Jul 16 22:09:28 2018 +0530
@@ -61,17 +61,21 @@
                                                 "WithFieldOfExplicitSelector$X.class").toString() };
         runCheck(params, new String [] {
 
-         "0: aload_0",
-         "1: iload_1",
-         "2: withfield     #2                  // Field i:I",
-         "5: astore_3",
-         "6: aload_3",
-         "7: aload_2",
-        "8: invokevirtual #3                  // Method java/lang/Integer.intValue:()I",
-        "11: withfield     #2                  // Field i:I",
-        "14: astore_3",
-        "15: aload_3",
-        "16: areturn"
+         "0: iload_1",
+         "1: aload_0",
+         "2: dup_x1",
+         "3: pop",
+         "4: withfield     #2                  // Field i:I",
+         "7: astore_3",
+         "8: aload_2",
+         "9: invokevirtual #3                  // Method java/lang/Integer.intValue:()I",
+        "12: aload_3",
+        "13: dup_x1",
+        "14: pop",
+        "15: withfield     #2                  // Field i:I",
+        "18: astore_3",
+        "19: aload_3",
+        "20: areturn"
          });
      }