Fix the side effects of clinit check

HClinitCheck obviously does reads so it's side effects should include
all reads and writes, just like HInvoke. GVN now explicitly allows
clinit check to be reused, which would otherwise be disallowed based on
the dependency introduced by the new side effects. Also make licm's
logic cleaner and treat clinit check as a special case also,
otherwise licm can't hoist clinit check due to the dependency
introduced by the new side effects also.

Test: run-test on host.
Change-Id: I16886cfe557803d84d84ce68fbb185ebfc0b84dc

4 files changed, 56 insertions, 8 deletions

diff --git a/compiler/optimizing/gvn.cc b/compiler/optimizing/gvn.cc
index 813772e9af..71c394ec1f 100644
--- a/compiler/optimizing/gvn.cc
+++ b/compiler/optimizing/gvn.cc
@@ -301,8 +301,11 @@ class ValueSet : public ArenaObject<kArenaAllocGvn> {
     // Pure instructions are put into odd buckets to speed up deletion. Note that in the
     // case of irreducible loops, we don't put pure instructions in odd buckets, as we
     // need to delete them when entering the loop.
-    if (instruction->GetSideEffects().HasDependencies() ||
-        instruction->GetBlock()->GetGraph()->HasIrreducibleLoops()) {
+    // ClinitCheck is treated as a pure instruction since it's only executed
+    // once.
+    bool pure = !instruction->GetSideEffects().HasDependencies() ||
+                instruction->IsClinitCheck();
+    if (!pure || instruction->GetBlock()->GetGraph()->HasIrreducibleLoops()) {
       return (hash_code << 1) | 0;
     } else {
       return (hash_code << 1) | 1;
diff --git a/compiler/optimizing/licm.cc b/compiler/optimizing/licm.cc
index 7af1a20f98..d3a0376e9c 100644
--- a/compiler/optimizing/licm.cc
+++ b/compiler/optimizing/licm.cc
@@ -129,10 +129,25 @@ void LICM::Run() {
            !inst_it.Done();
            inst_it.Advance()) {
         HInstruction* instruction = inst_it.Current();
-        if (instruction->CanBeMoved()
-            && (!instruction->CanThrow() || !found_first_non_hoisted_visible_instruction_in_loop)
-            && !instruction->GetSideEffects().MayDependOn(loop_effects)
-            && InputsAreDefinedBeforeLoop(instruction)) {
+        bool can_move = false;
+        if (instruction->CanBeMoved() && InputsAreDefinedBeforeLoop(instruction)) {
+          if (instruction->CanThrow()) {
+            if (!found_first_non_hoisted_visible_instruction_in_loop) {
+              DCHECK(instruction->GetBlock()->IsLoopHeader());
+              if (instruction->IsClinitCheck()) {
+                // clinit is only done once, and since all visible instructions
+                // in the loop header so far have been hoisted out, we can hoist
+                // the clinit check out also.
+                can_move = true;
+              } else if (!instruction->GetSideEffects().MayDependOn(loop_effects)) {
+                can_move = true;
+              }
+            }
+          } else if (!instruction->GetSideEffects().MayDependOn(loop_effects)) {
+            can_move = true;
+          }
+        }
+        if (can_move) {
           // We need to update the environment if the instruction has a loop header
           // phi in it.
           if (instruction->NeedsEnvironment()) {
@@ -142,7 +157,9 @@ void LICM::Run() {
           }
           instruction->MoveBefore(pre_header->GetLastInstruction());
           MaybeRecordStat(stats_, MethodCompilationStat::kLoopInvariantMoved);
-        } else if (instruction->CanThrow() || instruction->DoesAnyWrite()) {
+        }
+
+        if (!can_move && (instruction->CanThrow() || instruction->DoesAnyWrite())) {
           // If `instruction` can do something visible (throw or write),
           // we cannot move further instructions that can throw.
           found_first_non_hoisted_visible_instruction_in_loop = true;
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 42a9d95b9a..d33f2f1d65 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -6259,7 +6259,7 @@ class HClinitCheck FINAL : public HExpression<1> {
   HClinitCheck(HLoadClass* constant, uint32_t dex_pc)
       : HExpression(
             DataType::Type::kReference,
-            SideEffects::AllChanges(),  // Assume write/read on all fields/arrays.
+            SideEffects::AllExceptGCDependency(),  // Assume write/read on all fields/arrays.
             dex_pc) {
     SetRawInputAt(0, constant);
   }
diff --git a/test/445-checker-licm/src/Main.java b/test/445-checker-licm/src/Main.java
index 00ce3a9f8e..9635e70278 100644
--- a/test/445-checker-licm/src/Main.java
+++ b/test/445-checker-licm/src/Main.java
@@ -15,6 +15,11 @@
  */
 
 public class Main {
+  static class Dummy {
+    static int getValue() {
+      return 1;
+    }
+  }
 
   /// CHECK-START: int Main.div() licm (before)
   /// CHECK-DAG: Div loop:{{B\d+}}
@@ -107,6 +112,28 @@ public class Main {
     return result;
   }
 
+  /// CHECK-START: int Main.clinitCheck() licm (before)
+  /// CHECK-DAG: <<LoadClass:l\d+>> LoadClass loop:<<Loop:B\d+>>
+  /// CHECK-DAG:                    ClinitCheck [<<LoadClass>>] loop:<<Loop>>
+
+  /// CHECK-START: int Main.clinitCheck() licm (after)
+  /// CHECK-NOT:                    LoadClass loop:{{B\d+}}
+  /// CHECK-NOT:                    ClinitCheck loop:{{B\d+}}
+
+  /// CHECK-START: int Main.clinitCheck() licm (after)
+  /// CHECK-DAG: <<LoadClass:l\d+>> LoadClass loop:none
+  /// CHECK-DAG:                    ClinitCheck [<<LoadClass>>] loop:none
+
+  public static int clinitCheck() {
+    int i = 0;
+    int sum = 0;
+    do {
+      sum += Dummy.getValue();
+      i++;
+    } while (i < 10);
+    return sum;
+  }
+
   /// CHECK-START: int Main.divAndIntrinsic(int[]) licm (before)
   /// CHECK-DAG: Div loop:{{B\d+}}
 
@@ -213,6 +240,7 @@ public class Main {
     assertEquals(18900, innerMul());
     assertEquals(105, divByA(2, 0));
     assertEquals(12, arrayLength(new int[] { 4, 8 }));
+    assertEquals(10, clinitCheck());
     assertEquals(21, divAndIntrinsic(new int[] { 4, -2, 8, -3 }));
     assertEquals(45, invariantBoundIntrinsic(-10));
     assertEquals(30, invariantBodyIntrinsic(2, 3));