changeset 3300:ac1d5e333073

Summary: Opcode dup_x1 should not target value types or any type variables.
author sadayapalam
date Thu, 11 Feb 2016 18:55:07 +0530
parents 1b3a1239c255
children db795ec80b8c
files src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Items.java test/tools/javac/valhalla/values/CheckNoDups.java
diffstat 4 files changed, 87 insertions(+), 24 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java	Thu Feb 11 14:45:25 2016 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java	Thu Feb 11 18:55:07 2016 +0530
@@ -29,10 +29,12 @@
 import com.sun.tools.javac.code.Symbol.*;
 import com.sun.tools.javac.util.*;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
+import com.sun.tools.javac.jvm.Items.*;
 
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Optional;
+import java.util.function.Function;
 import java.util.function.Supplier;
 
 import static com.sun.tools.javac.code.TypeTag.BOT;
@@ -66,6 +68,12 @@
     public final boolean debugCode;
     public final boolean needStackMap;
 
+    /** disableValueDupsSwaps: When true, do not use dup, dup2, swap, dup_x1, dup_x2, dup2_x1 and dup2_x2
+     * when the top of stack comprises a value typed operand or `any' type variables. (prototype: we may
+     * end up doing this defensively for all operands in some places.)
+     */
+    private final boolean disableValueDupsSwaps;
+
     /**
      * Functional interface for decorating opcodes.
      */
@@ -221,7 +229,8 @@
                 CRTable crt,
                 Symtab syms,
                 Types types,
-                Pool pool) {
+                Pool pool,
+                boolean disableValueDupsSwaps) {
         this.meth = meth;
         this.fatcode = fatcode;
         this.lineMap = lineMap;
@@ -232,6 +241,7 @@
         this.types = types;
         this.debugCode = debugCode;
         this.stackMap = stackMap;
+        this.disableValueDupsSwaps = disableValueDupsSwaps;
         switch (stackMap) {
         case CLDC:
         case JSR202:
@@ -546,6 +556,40 @@
         postop();
     }
 
+    /** Emit an opcode with no operand field. For certain opcode & operand pairs,
+     * an equivalent opcode sequence may be emitted instead.
+     */
+    public void emitop0OrSequence(int op, Function<Type, Item> temporaryGenerator) {
+        final Type tos = state.peek();
+        if (!disableValueDupsSwaps || (!types.isValue(tos) && !types.isAnyTypeVar(tos))) {
+            emitop0(op);
+            return;
+        }
+        int limit = nextreg;
+        switch (op) {
+            case dup:
+                Item local = temporaryGenerator.apply(tos);
+                local.store();
+                local.load();
+                local.load();
+                break;
+            case dup_x1:
+                // Operand Stack: ..., value2, value1 => ..., value1, value2, value1
+                Item value1 = temporaryGenerator.apply(tos);
+                value1.store();
+                Item value2 = temporaryGenerator.apply(state.peek());
+                value2.store();
+                value1.load();
+                value2.load();
+                value1.load();
+                break;
+            default:
+                emitop0(op);
+                break;
+        }
+        endScopes(limit);
+    }
+
     /** Emit an opcode with no operand field.
      */
     public void emitop0(int op) {
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java	Thu Feb 11 14:45:25 2016 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java	Thu Feb 11 18:55:07 2016 +0530
@@ -1151,8 +1151,9 @@
                                                : null,
                                         syms,
                                         types,
-                                        pool.pool);
-            items = new Items(pool, code, syms, types, names, genericClassFile, disableValueDupsSwaps);
+                                        pool.pool,
+                                        disableValueDupsSwaps);
+            items = new Items(pool, code, syms, types, names, genericClassFile);
             if (code.debugCode) {
                 System.err.println(meth + " for body " + tree);
             }
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Items.java	Thu Feb 11 14:45:25 2016 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Items.java	Thu Feb 11 18:55:07 2016 +0530
@@ -82,18 +82,12 @@
     /** Switch: new constant pool mode support */
     boolean genericClassFile;
 
-    /** When true, do not use dup, dup2, swap, dup_x1, dup_x2, dup2_x1 and dup2_x2 when the top of stack
-     * comprises a value typed operand or `any' type variables. (prototype: we may end up doing this
-     * defensively for all operands in some places.)
-     */
-    boolean disableValueDupsSwaps;
-
     /** Items that exist only once (flyweight pattern).
      */
     private final Item voidItem;
     private final Item[] stackItem = new Item[TypeCodeCount];
 
-    public Items(PoolWriter pool, Code code, Symtab syms, Types types, Names names, boolean genericClassFile, boolean disableValueDupsSwaps) {
+    public Items(PoolWriter pool, Code code, Symtab syms, Types types, Names names, boolean genericClassFile) {
         this.code = code;
         this.pool = pool;
         this.types = types;
@@ -105,7 +99,6 @@
         stackItem[VOIDcode] = voidItem;
         this.syms = syms;
         this.names = names;
-        this.disableValueDupsSwaps = disableValueDupsSwaps;
     }
 
     /** Make a void item
@@ -723,17 +716,7 @@
             if (width() == 2) {
                 code.emitop0(dup2);
             } else {
-                final Type tos = code.state.peek();
-                if (disableValueDupsSwaps && (types.isValue(tos) || types.isAnyTypeVar(tos))) {
-                    int limit = code.nextreg;
-                    Item local = makeTempLocalItem(tos);
-                    local.store();
-                    local.load();
-                    local.load();
-                    code.endScopes(limit);
-                } else {
-                    code.emitop0(dup);
-                }
+                code.emitop0OrSequence(dup, Items.this::makeTempLocalItem);
             }
         }
 
@@ -742,8 +725,17 @@
         }
 
         void stash(int toscode) {
-            code.emitop0(
-                    (width() == 2 ? dup_x2 : dup_x1) + 3 * (Code.width(toscode) - 1));
+            final int toscodeWidth = Code.width(toscode);
+            if (width() == 2) {
+                code.emitop0(dup_x2 + 3 * (toscodeWidth - 1));
+            } else {
+                Assert.check(toscodeWidth == 1 || toscodeWidth == 2);
+                if (toscodeWidth == 1) {
+                    code.emitop0OrSequence(dup_x1, Items.this::makeTempLocalItem);
+                } else if (toscodeWidth == 2) {
+                    code.emitop0(dup2_x1);
+                }
+            }
         }
 
         int width() {
--- a/test/tools/javac/valhalla/values/CheckNoDups.java	Thu Feb 11 14:45:25 2016 +0530
+++ b/test/tools/javac/valhalla/values/CheckNoDups.java	Thu Feb 11 18:55:07 2016 +0530
@@ -65,6 +65,19 @@
         }
     }
 
+    public class DupX1Test {
+        final __ByValue class V {
+            final int x = 10;
+        }
+
+        V v;
+
+        void foo(int p) {
+            DupX1Test x;
+            foo(((x = new DupX1Test()).v = __Make V()).x);
+        }
+    }
+
     public static void main(String[] args) {
         new CheckNoDups().run();
     }
@@ -132,6 +145,19 @@
                         " 3: astore_1",
                         " 4: return",
                          });
+
+        params = new String [] { "-c",
+                                 Paths.get(System.getProperty("test.classes"),
+                                     "CheckNoDups$DupX1Test.class").toString() };
+
+        runCheck(params, new String [] {
+                        " 15: vnew          #25 ",
+                        " 18: vstore        3",
+                        " 20: astore        4",
+                        " 22: vload         3",
+                        " 24: aload         4",
+                        " 26: vload         3"
+                         });
      }
 
      void runCheck(String [] params, String [] expectedOut) {