8210066: [JVMCI] iterateFrames uses wrong GrowableArray API for appending
authordnsimon
Fri, 31 Aug 2018 11:43:06 +0200
changeset 51606 18afb2097ada
parent 51605 2ca553d34949
child 51607 5eb48e9d607a
8210066: [JVMCI] iterateFrames uses wrong GrowableArray API for appending Reviewed-by: dlong, twisti
src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java
--- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp	Fri Aug 31 09:37:03 2018 +0200
+++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp	Fri Aug 31 11:43:06 2018 +0200
@@ -1059,11 +1059,10 @@
               } else {
                 // some object might already have been re-allocated, only reallocate the non-allocated ones
                 objects = new GrowableArray<ScopeValue*>(scope->objects()->length());
-                int ii = 0;
                 for (int i = 0; i < scope->objects()->length(); i++) {
                   ObjectValue* sv = (ObjectValue*) scope->objects()->at(i);
                   if (sv->value().is_null()) {
-                    objects->at_put(ii++, sv);
+                    objects->append(sv);
                   }
                 }
               }
--- a/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java	Fri Aug 31 09:37:03 2018 +0200
+++ b/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java	Fri Aug 31 11:43:06 2018 +0200
@@ -47,6 +47,7 @@
  *                   -XX:CompileCommand=dontinline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame
  *                   -XX:CompileCommand=dontinline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame2
  *                   -XX:CompileCommand=inline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::recurse
+ *                   -XX:CompileCommand=inline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame3
  *                   -XX:+DoEscapeAnalysis -XX:-UseCounterDecay
  *                   -Dcompiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.materializeFirst=true
  *                   -Dcompiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.invalidate=false
@@ -58,6 +59,7 @@
  *                   -XX:CompileCommand=dontinline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame
  *                   -XX:CompileCommand=dontinline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame2
  *                   -XX:CompileCommand=inline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::recurse
+ *                   -XX:CompileCommand=inline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame3
  *                   -XX:+DoEscapeAnalysis -XX:-UseCounterDecay
  *                   -Dcompiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.materializeFirst=false
  *                   -Dcompiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.invalidate=false
@@ -69,6 +71,7 @@
  *                   -XX:CompileCommand=dontinline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame
  *                   -XX:CompileCommand=dontinline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame2
  *                   -XX:CompileCommand=inline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::recurse
+ *                   -XX:CompileCommand=inline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame3
  *                   -XX:+DoEscapeAnalysis -XX:-UseCounterDecay
  *                   -Dcompiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.materializeFirst=true
  *                   -Dcompiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.invalidate=true
@@ -80,6 +83,7 @@
  *                   -XX:CompileCommand=dontinline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame
  *                   -XX:CompileCommand=dontinline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame2
  *                   -XX:CompileCommand=inline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::recurse
+ *                   -XX:CompileCommand=inline,compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest::testFrame3
  *                   -XX:+DoEscapeAnalysis -XX:-UseCounterDecay
  *                   -Dcompiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.materializeFirst=false
  *                   -Dcompiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.invalidate=true
@@ -107,8 +111,11 @@
     private static final int COMPILE_THRESHOLD;
     private static final Method MATERIALIZED_METHOD;
     private static final Method NOT_MATERIALIZED_METHOD;
+    private static final Method FRAME3_METHOD;
     private static final ResolvedJavaMethod MATERIALIZED_RESOLVED;
     private static final ResolvedJavaMethod NOT_MATERIALIZED_RESOLVED;
+    private static final ResolvedJavaMethod FRAME2_RESOLVED;
+    private static final ResolvedJavaMethod FRAME3_RESOLVED;
     private static final boolean MATERIALIZE_FIRST;
 
     static {
@@ -120,13 +127,15 @@
                     String.class, int.class);
             method2 = MaterializeVirtualObjectTest.class.getDeclaredMethod("testFrame2",
                     String.class, int.class);
+            FRAME3_METHOD = MaterializeVirtualObjectTest.class.getDeclaredMethod("testFrame3",
+                    Helper.class, int.class);
         } catch (NoSuchMethodException e) {
             throw new Error("Can't get executable for test method", e);
         }
         ResolvedJavaMethod resolved1;
-        ResolvedJavaMethod resolved2;
         resolved1 = CTVMUtilities.getResolvedMethod(method1);
-        resolved2 = CTVMUtilities.getResolvedMethod(method2);
+        FRAME2_RESOLVED = CTVMUtilities.getResolvedMethod(method2);
+        FRAME3_RESOLVED = CTVMUtilities.getResolvedMethod(FRAME3_METHOD);
         INVALIDATE = Boolean.getBoolean(
                 "compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.invalidate");
         COMPILE_THRESHOLD = WB.getBooleanVMFlag("TieredCompilation")
@@ -134,8 +143,8 @@
                 : CompilerWhiteBoxTest.THRESHOLD * 2;
         MATERIALIZE_FIRST = Boolean.getBoolean(
                 "compiler.jvmci.compilerToVM.MaterializeVirtualObjectTest.materializeFirst");
-        MATERIALIZED_RESOLVED = MATERIALIZE_FIRST ? resolved1 : resolved2;
-        NOT_MATERIALIZED_RESOLVED = MATERIALIZE_FIRST ? resolved2 : resolved1;
+        MATERIALIZED_RESOLVED = MATERIALIZE_FIRST ? resolved1 : FRAME2_RESOLVED;
+        NOT_MATERIALIZED_RESOLVED = MATERIALIZE_FIRST ? FRAME2_RESOLVED : resolved1;
         MATERIALIZED_METHOD = MATERIALIZE_FIRST ? method1 : method2;
         NOT_MATERIALIZED_METHOD = MATERIALIZE_FIRST ? method2 : method1;
     }
@@ -171,6 +180,16 @@
         Asserts.assertTrue(WB.isMethodCompiled(NOT_MATERIALIZED_METHOD),
                 getName() + " : not materialized method not compiled");
         testFrame("someString", /* materialize */ CompilerWhiteBoxTest.THRESHOLD);
+
+        // run second test types
+        for (int i = 0; i < CompilerWhiteBoxTest.THRESHOLD; i++) {
+            testFrame("someString", i);
+        }
+        Asserts.assertTrue(WB.isMethodCompiled(MATERIALIZED_METHOD), getName()
+                + " : materialized method not compiled");
+        Asserts.assertTrue(WB.isMethodCompiled(NOT_MATERIALIZED_METHOD),
+                getName() + " : not materialized method not compiled");
+        testFrame("someString", /* materialize */ CompilerWhiteBoxTest.THRESHOLD + 1);
     }
 
     private void testFrame(String str, int iteration) {
@@ -178,13 +197,25 @@
         testFrame2(str, iteration);
         Asserts.assertTrue((helper.string != null) && (this != null)
                 && (helper != null), String.format("%s : some locals are null", getName()));
-     }
+    }
 
     private void testFrame2(String str, int iteration) {
         Helper helper = new Helper(str);
+        Helper helper2 = new Helper("bar");
+        testFrame3(helper, iteration);
+        Asserts.assertTrue((helper.string != null) && (this != null) && helper.string == str
+                && (helper != null), String.format("%s : some locals are null", getName()));
+        Asserts.assertTrue((helper2.string != null) && (this != null)
+                && (helper2 != null), String.format("%s : some locals are null", getName()));
+    }
+
+    private void testFrame3(Helper outerHelper, int iteration) {
+        Helper innerHelper = new Helper("foo");
         recurse(2, iteration);
-        Asserts.assertTrue((helper.string != null) && (this != null)
-                && (helper != null), String.format("%s : some locals are null", getName()));
+        Asserts.assertTrue((innerHelper.string != null) && (this != null)
+                && (innerHelper != null), String.format("%s : some locals are null", getName()));
+        Asserts.assertTrue((outerHelper.string != null) && (this != null)
+                && (outerHelper != null), String.format("%s : some locals are null", getName()));
     }
 
     private void recurse(int depth, int iteration) {
@@ -198,6 +229,48 @@
         }
     }
 
+    private void checkStructure(boolean materialize) {
+        boolean[] framesSeen = new boolean[2];
+        Object[] helpers = new Object[1];
+        CompilerToVMHelper.iterateFrames(
+            new ResolvedJavaMethod[] {FRAME3_RESOLVED},
+            null, /* any */
+            0,
+            f -> {
+                if (!framesSeen[1]) {
+                    Asserts.assertTrue(f.isMethod(FRAME3_RESOLVED),
+                            "Expected testFrame3 first");
+                    framesSeen[1] = true;
+                    Asserts.assertTrue(f.getLocal(0) != null, "this should not be null");
+                    Asserts.assertTrue(f.getLocal(1) != null, "outerHelper should not be null");
+                    Asserts.assertTrue(f.getLocal(3) != null, "innerHelper should not be null");
+                    Asserts.assertEQ(((Helper) f.getLocal(3)).string, "foo", "innerHelper.string should be foo");
+                    helpers[0] = f.getLocal(1);
+                    if (materialize) {
+                        f.materializeVirtualObjects(false);
+                    }
+                    return null; //continue
+                } else {
+                    Asserts.assertFalse(framesSeen[0], "frame3 can not have been seen");
+                    Asserts.assertTrue(f.isMethod(FRAME2_RESOLVED),
+                            "Expected testFrame2 second");
+                    framesSeen[0] = true;
+                    Asserts.assertTrue(f.getLocal(0) != null, "this should not be null");
+                    Asserts.assertTrue(f.getLocal(1) != null, "str should not be null");
+                    Asserts.assertTrue(f.getLocal(3) != null, "helper should not be null");
+                    Asserts.assertTrue(f.getLocal(4) != null, "helper2 should not be null");
+                    Asserts.assertEQ(((Helper) f.getLocal(3)).string, f.getLocal(1), "helper.string should be the same as str");
+                    Asserts.assertEQ(((Helper) f.getLocal(4)).string, "bar", "helper2.string should be foo");
+                    if (!materialize) {
+                        Asserts.assertEQ(f.getLocal(3), helpers[0], "helper should be the same as frame3's outerHelper");
+                    }
+                    return f; // stop
+                }
+            });
+        Asserts.assertTrue(framesSeen[1], "frame3 should have been seen");
+        Asserts.assertTrue(framesSeen[0], "frame2 should have been seen");
+    }
+
     private void check(int iteration) {
         // Materialize virtual objects on last invocation
         if (iteration == COMPILE_THRESHOLD) {
@@ -244,6 +317,9 @@
             // check that not materialized frame wasn't deoptimized
             Asserts.assertTrue(WB.isMethodCompiled(NOT_MATERIALIZED_METHOD), getName()
                     + " : not materialized method has unexpected compiled status");
+        } else if (iteration == COMPILE_THRESHOLD + 1) {
+            checkStructure(false);
+            checkStructure(true);
         }
     }