From 0a845200354f5dc3a3344c35823d2614cd5850ef Mon Sep 17 00:00:00 2001
From: Mingyao Yang <mingyao@google.com>
Date: Fri, 14 Oct 2016 16:26:08 -0700
Subject: [PATCH] More store/allocation elimination for singletons in case of
 loops

For a store into a singleton's location, if it happens inside a loop, it
means the singleton's location value may be killed by loop side effects.
However if the singleton is defined inside that loop, that loop should
be skipped since its loop side effects kill values at loop header where
the singleton's location doesn't exist yet.

Test: test-art-host

Bug: 31716107
Change-Id: Iae2494ea93295977f90d1463ee136a7e2e09ba9b
---
 compiler/optimizing/load_store_elimination.cc | 29 +++++++++++++++++++++------
 test/530-checker-lse/src/Main.java            | 29 +++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 734768683..820fa2959 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -168,7 +168,9 @@ class HeapLocation : public ArenaObject<kArenaAllocMisc> {
   const int16_t declaring_class_def_index_;  // declaring class's def's dex index.
   bool value_killed_by_loop_side_effects_;   // value of this location may be killed by loop
                                              // side effects because this location is stored
-                                             // into inside a loop.
+                                             // into inside a loop. This gives
+                                             // better info on whether a singleton's location
+                                             // value may be killed by loop side effects.
 
   DISALLOW_COPY_AND_ASSIGN(HeapLocation);
 };
@@ -420,8 +422,26 @@ class HeapLocationCollector : public HGraphVisitor {
   void VisitInstanceFieldSet(HInstanceFieldSet* instruction) OVERRIDE {
     HeapLocation* location = VisitFieldAccess(instruction->InputAt(0), instruction->GetFieldInfo());
     has_heap_stores_ = true;
-    if (instruction->GetBlock()->GetLoopInformation() != nullptr) {
-      location->SetValueKilledByLoopSideEffects(true);
+    if (location->GetReferenceInfo()->IsSingleton()) {
+      // A singleton's location value may be killed by loop side effects if it's
+      // defined before that loop, and it's stored into inside that loop.
+      HLoopInformation* loop_info = instruction->GetBlock()->GetLoopInformation();
+      if (loop_info != nullptr) {
+        HInstruction* ref = location->GetReferenceInfo()->GetReference();
+        DCHECK(ref->IsNewInstance());
+        if (loop_info->IsDefinedOutOfTheLoop(ref)) {
+          // ref's location value may be killed by this loop's side effects.
+          location->SetValueKilledByLoopSideEffects(true);
+        } else {
+          // ref is defined inside this loop so this loop's side effects cannot
+          // kill its location value at the loop header since ref/its location doesn't
+          // exist yet at the loop header.
+        }
+      }
+    } else {
+      // For non-singletons, value_killed_by_loop_side_effects_ is inited to
+      // true.
+      DCHECK_EQ(location->IsValueKilledByLoopSideEffects(), true);
     }
   }
 
@@ -810,9 +830,6 @@ class LSEVisitor : public HGraphVisitor {
         if (loop_info != nullptr) {
           // instruction is a store in the loop so the loop must does write.
           DCHECK(side_effects_.GetLoopEffects(loop_info->GetHeader()).DoesAnyWrite());
-          // If it's a singleton, IsValueKilledByLoopSideEffects() must be true.
-          DCHECK(!ref_info->IsSingleton() ||
-                 heap_location_collector_.GetHeapLocation(idx)->IsValueKilledByLoopSideEffects());
 
           if (loop_info->IsDefinedOutOfTheLoop(original_ref)) {
             DCHECK(original_ref->GetBlock()->Dominates(loop_info->GetPreHeader()));
diff --git a/test/530-checker-lse/src/Main.java b/test/530-checker-lse/src/Main.java
index 89875d7fe..6b0dedfe9 100644
--- a/test/530-checker-lse/src/Main.java
+++ b/test/530-checker-lse/src/Main.java
@@ -717,6 +717,33 @@ public class Main {
     return sumWithFilter(array, filter);
   }
 
+  private static int mI = 0;
+  private static float mF = 0f;
+
+  /// CHECK-START: float Main.testAllocationEliminationWithLoops() load_store_elimination (before)
+  /// CHECK: NewInstance
+  /// CHECK: NewInstance
+  /// CHECK: NewInstance
+
+  /// CHECK-START: float Main.testAllocationEliminationWithLoops() load_store_elimination (after)
+  /// CHECK-NOT: NewInstance
+
+  private static float testAllocationEliminationWithLoops() {
+    for (int i0 = 0; i0 < 5; i0++) {
+      for (int i1 = 0; i1 < 5; i1++) {
+        for (int i2 = 0; i2 < 5; i2++) {
+          int lI0 = ((int) new Integer(((int) new Integer(mI))));
+          if (((boolean) new Boolean(false))) {
+            for (int i3 = 576 - 1; i3 >= 0; i3--) {
+              mF -= 976981405.0f;
+            }
+          }
+        }
+      }
+    }
+    return 1.0f;
+  }
+
   static void assertIntEquals(int result, int expected) {
     if (expected != result) {
       throw new Error("Expected: " + expected + ", found: " + result);
@@ -779,6 +806,8 @@ public class Main {
     assertIntEquals($noinline$testHSelect(true), 0xdead);
     int[] array = {2, 5, 9, -1, -3, 10, 8, 4};
     assertIntEquals(sumWithinRange(array, 1, 5), 11);
+    assertFloatEquals(testAllocationEliminationWithLoops(), 1.0f);
+    assertFloatEquals(mF, 0f);
   }
 
   static boolean sFlag;
-- 
2.11.0