Revert^2 "Add support for try catches in LSE"

This reverts commit 2913ddad8b7f6e2ff7c92347ee8f67346cdfb16f.

Reason for revert: Updated tests

Bug: 227283233
Change-Id: Ied1e47b9cb55c2a39862e479b754f827e37e6303

9 files changed, 242 insertions, 32 deletions

diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index d42e4f7523..4651210d66 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -78,6 +78,8 @@
  *    all back-edges. We use Phi placeholders also for array heap locations with
  *    index defined inside the loop but this helps only when the value remains
  *    zero from the array allocation throughout the loop.
+ *  - For catch blocks, we clear all assumptions since we arrived due to an
+ *    instruction throwing.
  *  - For other basic blocks, we merge incoming values from the end of all
  *    predecessors. If any incoming value is unknown, the start value for this
  *    block is also unknown. Otherwise, if all the incoming values are the same
@@ -113,8 +115,6 @@
  *  - SIMD graphs (with VecLoad and VecStore instructions) are also handled. Any
  *    partial overlap access among ArrayGet/ArraySet/VecLoad/Store is seen as
  *    alias and no load/store is eliminated in such case.
- *  - Currently this LSE algorithm doesn't handle graph with try-catch, due to
- *    the special block merging structure.
  *
  * The time complexity of the initial phase has several components. The total
  * time for the initialization of heap values for all blocks is
@@ -1618,8 +1618,9 @@ void LSEVisitor::MergePredecessorRecords(HBasicBlock* block) {
   ScopedArenaVector<ValueRecord>& heap_values = heap_values_for_[block->GetBlockId()];
   DCHECK(heap_values.empty());
   size_t num_heap_locations = heap_location_collector_.GetNumberOfHeapLocations();
-  if (block->GetPredecessors().empty()) {
-    DCHECK(block->IsEntryBlock());
+  if (block->GetPredecessors().empty() || (block->GetTryCatchInformation() != nullptr &&
+                                           block->GetTryCatchInformation()->IsCatchBlock())) {
+    DCHECK_IMPLIES(block->GetPredecessors().empty(), block->IsEntryBlock());
     heap_values.resize(num_heap_locations,
                        {/*value=*/Value::PureUnknown(), /*stored_by=*/Value::PureUnknown()});
     return;
@@ -3877,9 +3878,8 @@ class LSEVisitorWrapper : public DeletableArenaObject<kArenaAllocLSE> {
 };
 
 bool LoadStoreElimination::Run(bool enable_partial_lse) {
-  if (graph_->IsDebuggable() || graph_->HasTryCatch()) {
+  if (graph_->IsDebuggable()) {
     // Debugger may set heap values or trigger deoptimization of callers.
-    // Try/catch support not implemented yet.
     // Skip this optimization.
     return false;
   }
diff --git a/test/004-ReferenceMap/stack_walk_refmap_jni.cc b/test/004-ReferenceMap/stack_walk_refmap_jni.cc
index 4317b5d0ba..0659c0beb5 100644
--- a/test/004-ReferenceMap/stack_walk_refmap_jni.cc
+++ b/test/004-ReferenceMap/stack_walk_refmap_jni.cc
@@ -52,7 +52,6 @@ struct ReferenceMap2Visitor : public CheckReferenceMapVisitor {
     // we know the Dex registers with live reference values. Assert that what we
     // find is what is expected.
     if (m_name.compare("f") == 0) {
-      CHECK_REGS_CONTAIN_REFS(0x03U, true, 8);  // v8: this
       CHECK_REGS_CONTAIN_REFS(0x06U, true, 8, 1);  // v8: this, v1: x
       CHECK_REGS_CONTAIN_REFS(0x0cU, true, 8, 3, 1);  // v8: this, v3: y, v1: x
       CHECK_REGS_CONTAIN_REFS(0x10U, true, 8, 3, 1);  // v8: this, v3: y, v1: x
diff --git a/test/061-out-of-memory/src/Main.java b/test/061-out-of-memory/src/Main.java
index bda978eb2c..a9d5593390 100644
--- a/test/061-out-of-memory/src/Main.java
+++ b/test/061-out-of-memory/src/Main.java
@@ -23,25 +23,25 @@ import java.util.LinkedList;
 public class Main {
     public static void main(String args[]) {
         System.out.println("tests beginning");
-        testHugeArray();
-        testOomeLarge();
-        testOomeSmall();
-        testOomeToCharArray();
+        $noinline$testHugeArray();
+        $noinline$testOomeLarge();
+        $noinline$testOomeSmall();
+        $noinline$testOomeToCharArray();
         System.out.println("tests succeeded");
     }
 
-    private static void testHugeArray() {
+    private static int[] $noinline$testHugeArray() {
+        int[] tooBig = null;
         try {
             final int COUNT = 32768*32768 + 4;
-            int[] tooBig = new int[COUNT];
-
-            Arrays.fill(tooBig, 0xdd);
+            tooBig = new int[COUNT];
         } catch (OutOfMemoryError oom) {
             System.out.println("Got expected huge-array OOM");
         }
+        return tooBig;
     }
 
-    private static void testOomeLarge() {
+    private static void $noinline$testOomeLarge() {
         System.out.println("testOomeLarge beginning");
 
         Boolean sawEx = false;
@@ -52,7 +52,6 @@ public class Main {
             // try to allocate it instead of short-circuiting.
             a = new byte[(int) Runtime.getRuntime().maxMemory() - 32];
         } catch (OutOfMemoryError oom) {
-            //Log.i(TAG, "HeapTest/OomeLarge caught " + oom);
             sawEx = true;
         }
 
@@ -96,7 +95,7 @@ public class Main {
         return true;
     }
 
-    private static void testOomeSmall() {
+    private static void $noinline$testOomeSmall() {
         System.out.println("testOomeSmall beginning");
         if (!testOomeSmallInternal()) {
             /* Can't reliably throw this from inside the internal function, because
@@ -108,7 +107,7 @@ public class Main {
         System.out.println("testOomeSmall succeeded");
     }
 
-    private static void testOomeToCharArray() {
+    private static Object $noinline$testOomeToCharArray() {
         Object[] o = new Object[2000000];
         String test = "test";
         int i = 0;
@@ -123,5 +122,6 @@ public class Main {
             o = null;
             System.out.println("Got expected toCharArray OOM");
         }
+        return o;
     }
 }
diff --git a/test/510-checker-try-catch/src/Main.java b/test/510-checker-try-catch/src/Main.java
index 5d1d2d8f85..2877ef88af 100644
--- a/test/510-checker-try-catch/src/Main.java
+++ b/test/510-checker-try-catch/src/Main.java
@@ -45,16 +45,13 @@ public class Main {
   /// CHECK-DAG: <<ArrayA:l\d+>>       ParameterValue
   /// CHECK-DAG: <<ArrayB:l\d+>>       ParameterValue
   //
-
   /// CHECK-DAG: <<NullCh1:l\d+>>      NullCheck [<<ArrayA>>]
   /// CHECK-DAG: <<LengthA:i\d+>>      ArrayLength
   /// CHECK-DAG: <<BoundsCh1:i\d+>>    BoundsCheck [<<IndexParam>>,<<LengthA>>]
-  /// CHECK-DAG: <<IntAddr1:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
-  /// CHECK-DAG:                       ArraySet [<<IntAddr1>>,<<BoundsCh1>>,<<Const1>>]
   /// CHECK-DAG:                       TryBoundary
   //
-  /// CHECK-DAG: <<IntAddr2:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
-  /// CHECK-DAG:                       ArraySet [<<IntAddr2>>,<<BoundsCh1>>,<<Const2>>]
+  /// CHECK-DAG: <<IntAddr1:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
+  /// CHECK-DAG:                       ArraySet [<<IntAddr1>>,<<BoundsCh1>>,<<Const2>>]
   /// CHECK-DAG: <<NullChB:l\d+>>      NullCheck [<<ArrayB>>]
   /// CHECK-DAG: <<LengthB:i\d+>>      ArrayLength
   /// CHECK-DAG: <<BoundsChB:i\d+>>    BoundsCheck [<<Const0>>,<<LengthB>>]
@@ -63,13 +60,13 @@ public class Main {
   /// CHECK-DAG: <<Div:i\d+>>          Div [<<GetB>>,<<ZeroCheck>>]
   /// CHECK-DAG: <<Xplus1:i\d+>>       Add [<<IndexParam>>,<<Const1>>]
   /// CHECK-DAG: <<BoundsCh2:i\d+>>    BoundsCheck [<<Xplus1>>,<<LengthA>>]
-  /// CHECK-DAG: <<IntAddr3:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
-  /// CHECK-DAG:                       ArraySet [<<IntAddr3>>,<<BoundsCh2>>,<<Div>>]
+  /// CHECK-DAG: <<IntAddr2:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
+  /// CHECK-DAG:                       ArraySet [<<IntAddr2>>,<<BoundsCh2>>,<<Div>>]
   /// CHECK-DAG:                       TryBoundary
   //
   /// CHECK-DAG:                       ClearException
-  /// CHECK-DAG: <<IntAddr4:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
-  /// CHECK-DAG:                       ArraySet [<<IntAddr4>>,<<BoundsCh1>>,<<Const1>>]
+  /// CHECK-DAG: <<IntAddr3:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
+  /// CHECK-DAG:                       ArraySet [<<IntAddr3>>,<<BoundsCh1>>,<<Const1>>]
   //
   /// CHECK-NOT:                       NullCheck
   /// CHECK-NOT:                       IntermediateAddress
@@ -86,10 +83,9 @@ public class Main {
   /// CHECK-DAG: <<NullCh1:l\d+>>      NullCheck [<<ArrayA>>]
   /// CHECK-DAG: <<LengthA:i\d+>>      ArrayLength
   /// CHECK-DAG: <<BoundsCh1:i\d+>>    BoundsCheck [<<IndexParam>>,<<LengthA>>]
-  /// CHECK-DAG: <<IntAddr1:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
-  /// CHECK-DAG:                       ArraySet [<<IntAddr1>>,<<BoundsCh1>>,<<Const1>>]
   /// CHECK-DAG:                       TryBoundary
   //
+  /// CHECK-DAG: <<IntAddr1:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
   /// CHECK-DAG:                       ArraySet [<<IntAddr1>>,<<BoundsCh1>>,<<Const2>>]
   /// CHECK-DAG: <<NullChB:l\d+>>      NullCheck [<<ArrayB>>]
   /// CHECK-DAG: <<LengthB:i\d+>>      ArrayLength
@@ -103,14 +99,16 @@ public class Main {
   /// CHECK-DAG:                       TryBoundary
   //
   /// CHECK-DAG:                       ClearException
-  /// CHECK-DAG: <<IntAddr4:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
-  /// CHECK-DAG:                       ArraySet [<<IntAddr4>>,<<BoundsCh1>>,<<Const1>>]
+  /// CHECK-DAG: <<IntAddr3:i\d+>>     IntermediateAddress [<<NullCh1>>,<<Offset>>]
+  /// CHECK-DAG:                       ArraySet [<<IntAddr3>>,<<BoundsCh1>>,<<Const1>>]
   //
   /// CHECK-NOT:                       NullCheck
   /// CHECK-NOT:                       IntermediateAddress
 
   //  Make sure that BoundsCheck, DivZeroCheck and NullCheck don't stop IntermediateAddress sharing.
   public static void boundsCheckAndCatch(int x, int[] a, int[] b) {
+    // This a[x] = 1 will be eliminated by LSE, but its related instructions (e.g. NullCheck and
+    // BoundsCheck) will remain.
     a[x] = 1;
     try {
       a[x] = 2;
diff --git a/test/530-checker-lse-try-catch/expected-stderr.txt b/test/530-checker-lse-try-catch/expected-stderr.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/530-checker-lse-try-catch/expected-stderr.txt
diff --git a/test/530-checker-lse-try-catch/expected-stdout.txt b/test/530-checker-lse-try-catch/expected-stdout.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/530-checker-lse-try-catch/expected-stdout.txt
diff --git a/test/530-checker-lse-try-catch/info.txt b/test/530-checker-lse-try-catch/info.txt
new file mode 100644
index 0000000000..9ceef8275a
--- /dev/null
+++ b/test/530-checker-lse-try-catch/info.txt
@@ -0,0 +1 @@
+Checker test for testing load-store elimination for try catches.
diff --git a/test/530-checker-lse-try-catch/src/Main.java b/test/530-checker-lse-try-catch/src/Main.java
new file mode 100644
index 0000000000..c7db02a668
--- /dev/null
+++ b/test/530-checker-lse-try-catch/src/Main.java
@@ -0,0 +1,207 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class Point {
+  int x;
+  int y;
+}
+
+public class Main {
+  public static void main(String[] args) {
+    final boolean boolean_throw = false;
+    final boolean boolean_other_throw = false;
+    assertEquals(3,
+        $noinline$testDifferentFields(
+            new Point(), new Point(), boolean_throw, boolean_other_throw));
+    assertEquals(1, $noinline$testRedundantStore(new Point(), boolean_throw, boolean_other_throw));
+    assertEquals(1, $noinline$testTryCatchBlocking(new Point(), boolean_throw));
+    assertEquals(1, $noinline$testTryCatchPhi(new Point(), boolean_throw));
+    assertEquals(2, $noinline$testTryCatchPhiWithTwoCatches(new Point(), new int[0]));
+  }
+
+  /// CHECK-START: int Main.$noinline$testDifferentFields(Point, Point, boolean, boolean) load_store_elimination (before)
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.x
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldGet field_name:Point.x
+  /// CHECK-DAG:     InstanceFieldGet field_name:Point.y
+
+  /// CHECK-START: int Main.$noinline$testDifferentFields(Point, Point, boolean, boolean) load_store_elimination (after)
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.x
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+
+  /// CHECK-START: int Main.$noinline$testDifferentFields(Point, Point, boolean, boolean) load_store_elimination (after)
+  /// CHECK-NOT:     InstanceFieldGet field_name:Point.x
+  /// CHECK-NOT:     InstanceFieldGet field_name:Point.y
+
+  // Consistency check to make sure the try/catches weren't removed by an earlier pass.
+  /// CHECK-START: int Main.$noinline$testDifferentFields(Point, Point, boolean, boolean) load_store_elimination (after)
+  /// CHECK:         TryBoundary kind:entry
+  /// CHECK:         TryBoundary kind:entry
+
+  // Different fields shouldn't alias.
+  private static int $noinline$testDifferentFields(
+      Point obj1, Point obj2, boolean boolean_throw, boolean boolean_other_throw) {
+    try {
+      if (boolean_throw) {
+        throw new Error();
+      }
+    } catch (Error e) {
+      return 0;
+    }
+    obj1.x = 1;
+    obj2.y = 2;
+    int result = obj1.x + obj2.y;
+    try {
+      if (boolean_other_throw) {
+        throw new Error();
+      }
+    } catch (Error e) {
+      return 0;
+    }
+    return result;
+  }
+
+  /// CHECK-START: int Main.$noinline$testRedundantStore(Point, boolean, boolean) load_store_elimination (before)
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldGet field_name:Point.y
+
+  /// CHECK-START: int Main.$noinline$testRedundantStore(Point, boolean, boolean) load_store_elimination (after)
+  /// CHECK:         InstanceFieldSet field_name:Point.y
+  /// CHECK-NOT:     InstanceFieldSet field_name:Point.y
+
+  /// CHECK-START: int Main.$noinline$testRedundantStore(Point, boolean, boolean) load_store_elimination (after)
+  /// CHECK-NOT:     InstanceFieldGet field_name:Point.y
+
+  // Consistency check to make sure the try/catches weren't removed by an earlier pass.
+  /// CHECK-START: int Main.$noinline$testRedundantStore(Point, boolean, boolean) load_store_elimination (after)
+  /// CHECK-DAG:     TryBoundary kind:entry
+  /// CHECK-DAG:     TryBoundary kind:entry
+
+  // Redundant store of the same value.
+  private static int $noinline$testRedundantStore(
+      Point obj, boolean boolean_throw, boolean boolean_other_throw) {
+    try {
+      if (boolean_throw) {
+        throw new Error();
+      }
+    } catch (Error e) {
+      return 0;
+    }
+    obj.y = 1;
+    obj.y = 1;
+    try {
+      if (boolean_other_throw) {
+        throw new Error();
+      }
+    } catch (Error e) {
+      return 0;
+    }
+    return obj.y;
+  }
+
+  /// CHECK-START: int Main.$noinline$testTryCatchBlocking(Point, boolean) load_store_elimination (before)
+  /// CHECK: InstanceFieldSet field_name:Point.y
+  /// CHECK: InstanceFieldGet field_name:Point.y
+
+  /// CHECK-START: int Main.$noinline$testTryCatchBlocking(Point, boolean) load_store_elimination (after)
+  /// CHECK: InstanceFieldSet field_name:Point.y
+  /// CHECK: InstanceFieldGet field_name:Point.y
+
+  // Consistency check to make sure the try/catch wasn't removed by an earlier pass.
+  /// CHECK-START: int Main.$noinline$testTryCatchBlocking(Point, boolean) load_store_elimination (after)
+  /// CHECK: TryBoundary kind:entry
+
+  // We cannot remove the Get since we might have thrown.
+  private static int $noinline$testTryCatchBlocking(Point obj, boolean boolean_throw) {
+    obj.y = 1;
+    try {
+      if (boolean_throw) {
+        throw new Error();
+      }
+    } catch (Error e) {
+    }
+    return obj.y;
+  }
+
+  /// CHECK-START: int Main.$noinline$testTryCatchPhi(Point, boolean) load_store_elimination (before)
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldGet field_name:Point.y
+
+  /// CHECK-START: int Main.$noinline$testTryCatchPhi(Point, boolean) load_store_elimination (after)
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+
+  /// CHECK-START: int Main.$noinline$testTryCatchPhi(Point, boolean) load_store_elimination (after)
+  /// CHECK-NOT:     InstanceFieldGet field_name:Point.y
+
+  // Consistency check to make sure the try/catch wasn't removed by an earlier pass.
+  /// CHECK-START: int Main.$noinline$testTryCatchPhi(Point, boolean) load_store_elimination (after)
+  /// CHECK-DAG:     TryBoundary kind:entry
+
+  // We either threw and we set the value in the catch, or we didn't throw and we set the value
+  // before the catch. We can solve that with a Phi and skip the get.
+  private static int $noinline$testTryCatchPhi(Point obj, boolean boolean_throw) {
+    obj.y = 1;
+    try {
+      if (boolean_throw) {
+        throw new Error();
+      }
+    } catch (Error e) {
+      obj.y = 2;
+    }
+    return obj.y;
+  }
+
+
+  /// CHECK-START: int Main.$noinline$testTryCatchPhiWithTwoCatches(Point, int[]) load_store_elimination (before)
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldGet field_name:Point.y
+
+  /// CHECK-START: int Main.$noinline$testTryCatchPhiWithTwoCatches(Point, int[]) load_store_elimination (after)
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+  /// CHECK-DAG:     InstanceFieldSet field_name:Point.y
+
+  /// CHECK-START: int Main.$noinline$testTryCatchPhiWithTwoCatches(Point, int[]) load_store_elimination (after)
+  /// CHECK-NOT: InstanceFieldGet field_name:Point.y
+
+  // Consistency check to make sure the try/catch wasn't removed by an earlier pass.
+  /// CHECK-START: int Main.$noinline$testTryCatchPhiWithTwoCatches(Point, int[]) load_store_elimination (after)
+  /// CHECK-DAG: TryBoundary kind:entry
+  private static int $noinline$testTryCatchPhiWithTwoCatches(Point obj, int[] numbers) {
+    obj.y = 1;
+    try {
+      if (numbers[0] == 1) {
+        throw new Error();
+      }
+    } catch (ArrayIndexOutOfBoundsException e) {
+      obj.y = 2;
+    } catch (Error e) {
+      obj.y = 3;
+    }
+    return obj.y;
+  }
+
+  private static void assertEquals(int expected, int actual) {
+    if (expected != actual) {
+      throw new AssertionError("Expected: " + expected + ", Actual: " + actual);
+    }
+  }
+}
diff --git a/test/639-checker-code-sinking/src/Main.java b/test/639-checker-code-sinking/src/Main.java
index 743d50148c..5e465f2716 100644
--- a/test/639-checker-code-sinking/src/Main.java
+++ b/test/639-checker-code-sinking/src/Main.java
@@ -679,6 +679,11 @@ public class Main {
       }
     } catch (NullPointerException e) {
     }
+
+    // We want to use obj over here so that it doesn't get deleted by LSE.
+    if (obj.x == 123) {
+      return 123;
+    }
     return x;
   }