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

diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 7347686..820fa29 100644 (file)
--- 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 89875d7..6b0dedf 100644 (file)
--- 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;