Fix merging HLoadClass with HNewInstance.

Do not merge HLoadClass with HNewInstance if they do not
come from the same dex instruction. This is the same check
as for moving the clinit check responsibility around.

Test: Added a regression test, run standard ART test suite
plus gcstress and ndebug modes on host and Nexus 9.

Bug: 29570861
Bug: 29321958

(cherry picked from commit c7591b4c0dd405a766f4d701deea6c3750101978)

Change-Id: I0f019c36dd991889caad2666e61bbcc6d9d9c91f

diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc
index dcc89e8..38e42f7 100644 (file)
--- a/compiler/optimizing/prepare_for_register_allocation.cc
+++ b/compiler/optimizing/prepare_for_register_allocation.cc
@@ -130,7 +130,11 @@ void PrepareForRegisterAllocation::VisitNewInstance(HNewInstance* instruction) {
     instruction->ReplaceInput(GetGraph()->GetIntConstant(load_class->GetTypeIndex()), 0);
     // The allocation entry point that deals with access checks does not work with inlined
     // methods, so we need to check whether this allocation comes from an inlined method.
-    if (has_only_one_use && !instruction->GetEnvironment()->IsFromInlinedInvoke()) {
+    // We also need to make the same check as for moving clinit check, whether the HLoadClass
+    // has the clinit check responsibility or not (HLoadClass can throw anyway).
+    if (has_only_one_use &&
+        !instruction->GetEnvironment()->IsFromInlinedInvoke() &&
+        CanMoveClinitCheck(load_class, instruction)) {
       // We can remove the load class from the graph. If it needed access checks, we delegate
       // the access check to the allocation.
       if (load_class->NeedsAccessCheck()) {
@@ -188,7 +192,8 @@ bool PrepareForRegisterAllocation::CanMoveClinitCheck(HInstruction* input,
                                                       HInstruction* user) const {
   // Determine if input and user come from the same dex instruction, so that we can move
   // the clinit check responsibility from one to the other, i.e. from HClinitCheck (user)
-  // to HLoadClass (input), or from HClinitCheck (input) to HInvokeStaticOrDirect (user).
+  // to HLoadClass (input), or from HClinitCheck (input) to HInvokeStaticOrDirect (user),
+  // or from HLoadClass (input) to HNewInstance (user).
 
   // Start with a quick dex pc check.
   if (user->GetDexPc() != input->GetDexPc()) {

diff --git a/test/478-checker-clinit-check-pruning/expected.txt b/test/478-checker-clinit-check-pruning/expected.txt
index 7de097f..6f73b65 100644 (file)
--- a/test/478-checker-clinit-check-pruning/expected.txt
+++ b/test/478-checker-clinit-check-pruning/expected.txt
@@ -10,3 +10,4 @@ Main$ClassWithClinit9's static initializer
 Main$ClassWithClinit10's static initializer
 Main$ClassWithClinit11's static initializer
 Main$ClassWithClinit12's static initializer
+Main$ClassWithClinit13's static initializer

diff --git a/test/478-checker-clinit-check-pruning/src/Main.java b/test/478-checker-clinit-check-pruning/src/Main.java
index 7993513..6fc12f1 100644 (file)
--- a/test/478-checker-clinit-check-pruning/src/Main.java
+++ b/test/478-checker-clinit-check-pruning/src/Main.java
@@ -16,6 +16,8 @@
 
 public class Main {
 
+  static boolean doThrow = false;
+
   /*
    * Ensure an inlined static invoke explicitly triggers the
    * initialization check of the called method's declaring class, and
@@ -310,12 +312,12 @@ public class Main {
   /// CHECK-START: void Main.constClassAndInvokeStatic(java.lang.Iterable) liveness (before)
   /// CHECK-NOT:                           ClinitCheck
 
-  static void constClassAndInvokeStatic(Iterable it) {
+  static void constClassAndInvokeStatic(Iterable<?> it) {
     $opt$inline$ignoreClass(ClassWithClinit7.class);
     ClassWithClinit7.someStaticMethod(it);
   }
 
-  static void $opt$inline$ignoreClass(Class c) {
+  static void $opt$inline$ignoreClass(Class<?> c) {
   }
 
   static class ClassWithClinit7 {
@@ -324,7 +326,7 @@ public class Main {
     }
 
     // Note: not inlined from constClassAndInvokeStatic() but fully inlined from main().
-    static void someStaticMethod(Iterable it) {
+    static void someStaticMethod(Iterable<?> it) {
       // We're not inlining invoke-interface at the moment.
       it.iterator();
     }
@@ -341,7 +343,7 @@ public class Main {
   /// CHECK-START: void Main.sgetAndInvokeStatic(java.lang.Iterable) liveness (before)
   /// CHECK-NOT:                           ClinitCheck
 
-  static void sgetAndInvokeStatic(Iterable it) {
+  static void sgetAndInvokeStatic(Iterable<?> it) {
     $opt$inline$ignoreInt(ClassWithClinit8.value);
     ClassWithClinit8.someStaticMethod(it);
   }
@@ -356,7 +358,7 @@ public class Main {
     }
 
     // Note: not inlined from sgetAndInvokeStatic() but fully inlined from main().
-    static void someStaticMethod(Iterable it) {
+    static void someStaticMethod(Iterable<?> it) {
       // We're not inlining invoke-interface at the moment.
       it.iterator();
     }
@@ -372,7 +374,7 @@ public class Main {
   /// CHECK:                               ClinitCheck
   /// CHECK:                               InvokeStaticOrDirect clinit_check:none
 
-  static void constClassSgetAndInvokeStatic(Iterable it) {
+  static void constClassSgetAndInvokeStatic(Iterable<?> it) {
     $opt$inline$ignoreClass(ClassWithClinit9.class);
     $opt$inline$ignoreInt(ClassWithClinit9.value);
     ClassWithClinit9.someStaticMethod(it);
@@ -385,7 +387,7 @@ public class Main {
     }
 
     // Note: not inlined from constClassSgetAndInvokeStatic() but fully inlined from main().
-    static void someStaticMethod(Iterable it) {
+    static void someStaticMethod(Iterable<?> it) {
       // We're not inlining invoke-interface at the moment.
       it.iterator();
     }
@@ -403,12 +405,12 @@ public class Main {
   /// CHECK-START: void Main.inlinedInvokeStaticViaNonStatic(java.lang.Iterable) liveness (before)
   /// CHECK-NOT:                           ClinitCheck
 
-  static void inlinedInvokeStaticViaNonStatic(Iterable it) {
+  static void inlinedInvokeStaticViaNonStatic(Iterable<?> it) {
     inlinedInvokeStaticViaNonStaticHelper(null);
     inlinedInvokeStaticViaNonStaticHelper(it);
   }
 
-  static void inlinedInvokeStaticViaNonStaticHelper(Iterable it) {
+  static void inlinedInvokeStaticViaNonStaticHelper(Iterable<?> it) {
     ClassWithClinit10.inlinedForNull(it);
   }
 
@@ -418,7 +420,7 @@ public class Main {
       System.out.println("Main$ClassWithClinit10's static initializer");
     }
 
-    static void inlinedForNull(Iterable it) {
+    static void inlinedForNull(Iterable<?> it) {
       if (it != null) {
         // We're not inlining invoke-interface at the moment.
         it.iterator();
@@ -443,7 +445,7 @@ public class Main {
   /// CHECK-START: void Main.inlinedInvokeStaticViaStatic(java.lang.Iterable) liveness (before)
   /// CHECK-NOT:                           ClinitCheck
 
-  static void inlinedInvokeStaticViaStatic(Iterable it) {
+  static void inlinedInvokeStaticViaStatic(Iterable<?> it) {
     ClassWithClinit11.callInlinedForNull(it);
   }
 
@@ -453,11 +455,11 @@ public class Main {
       System.out.println("Main$ClassWithClinit11's static initializer");
     }
 
-    static void callInlinedForNull(Iterable it) {
+    static void callInlinedForNull(Iterable<?> it) {
       inlinedForNull(it);
     }
 
-    static void inlinedForNull(Iterable it) {
+    static void inlinedForNull(Iterable<?> it) {
       // We're not inlining invoke-interface at the moment.
       it.iterator();
     }
@@ -475,7 +477,7 @@ public class Main {
   /// CHECK-START: void Main.inlinedInvokeStaticViaStaticTwice(java.lang.Iterable) liveness (before)
   /// CHECK-NOT:                           ClinitCheck
 
-  static void inlinedInvokeStaticViaStaticTwice(Iterable it) {
+  static void inlinedInvokeStaticViaStaticTwice(Iterable<?> it) {
     ClassWithClinit12.callInlinedForNull(null);
     ClassWithClinit12.callInlinedForNull(it);
   }
@@ -486,11 +488,11 @@ public class Main {
       System.out.println("Main$ClassWithClinit12's static initializer");
     }
 
-    static void callInlinedForNull(Iterable it) {
+    static void callInlinedForNull(Iterable<?> it) {
       inlinedForNull(it);
     }
 
-    static void inlinedForNull(Iterable it) {
+    static void inlinedForNull(Iterable<?> it) {
       if (it != null) {
         // We're not inlining invoke-interface at the moment.
         it.iterator();
@@ -498,6 +500,28 @@ public class Main {
     }
   }
 
+  static class ClassWithClinit13 {
+    static {
+      System.out.println("Main$ClassWithClinit13's static initializer");
+    }
+
+    public static void $inline$forwardToGetIterator(Iterable<?> it) {
+      $noinline$getIterator(it);
+    }
+
+    public static void $noinline$getIterator(Iterable<?> it) {
+      // We're not inlining invoke-interface at the moment.
+      it.iterator();
+    }
+  }
+
+  // TODO: Write checker statements.
+  static Object $noinline$testInliningAndNewInstance(Iterable<?> it) {
+    if (doThrow) { throw new Error(); }
+    ClassWithClinit13.$inline$forwardToGetIterator(it);
+    return new ClassWithClinit13();
+  }
+
   // TODO: Add a test for the case of a static method whose declaring
   // class type index is not available (i.e. when `storage_index`
   // equals `DexFile::kDexNoIndex` in
@@ -517,5 +541,6 @@ public class Main {
     inlinedInvokeStaticViaNonStatic(it);
     inlinedInvokeStaticViaStatic(it);
     inlinedInvokeStaticViaStaticTwice(it);
+    $noinline$testInliningAndNewInstance(it);
   }
 }