summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Santiago Aboy Solanes <solanes@google.com> 2022-04-08 09:02:05 +0000
committer Santiago Aboy Solanes <solanes@google.com> 2022-04-08 09:07:54 +0000
commit2913ddad8b7f6e2ff7c92347ee8f67346cdfb16f (patch)
tree744d6ca4b0e60debfb8abe47592bc56e4269f7b6
parentfb95eedb3c8a383ea94868f5da14b5ea9b69a87d (diff)
Revert "Add support for try catches in LSE"
This reverts commit 62b12658b77536304fc9b0b07150a6500896b354. Reason for revert: Bot failures e.g. https://ci.chromium.org/ui/p/art/builders/ci/angler-armv7-non-gen-cc/1961/overview Change-Id: I35357794974aa099d1c14562c8c5313d795989f9
-rw-r--r--compiler/optimizing/load_store_elimination.cc12
-rw-r--r--test/004-ReferenceMap/stack_walk_refmap_jni.cc1
-rw-r--r--test/061-out-of-memory/src/Main.java3
-rw-r--r--test/530-checker-lse-try-catch/expected-stderr.txt0
-rw-r--r--test/530-checker-lse-try-catch/expected-stdout.txt0
-rw-r--r--test/530-checker-lse-try-catch/info.txt1
-rw-r--r--test/530-checker-lse-try-catch/src/Main.java207
-rw-r--r--test/639-checker-code-sinking/src/Main.java5
8 files changed, 8 insertions, 221 deletions
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 4651210d66..d42e4f7523 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -78,8 +78,6 @@
* 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
@@ -115,6 +113,8 @@
* - 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,9 +1618,8 @@ 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() || (block->GetTryCatchInformation() != nullptr &&
- block->GetTryCatchInformation()->IsCatchBlock())) {
- DCHECK_IMPLIES(block->GetPredecessors().empty(), block->IsEntryBlock());
+ if (block->GetPredecessors().empty()) {
+ DCHECK(block->IsEntryBlock());
heap_values.resize(num_heap_locations,
{/*value=*/Value::PureUnknown(), /*stored_by=*/Value::PureUnknown()});
return;
@@ -3878,8 +3877,9 @@ class LSEVisitorWrapper : public DeletableArenaObject<kArenaAllocLSE> {
};
bool LoadStoreElimination::Run(bool enable_partial_lse) {
- if (graph_->IsDebuggable()) {
+ if (graph_->IsDebuggable() || graph_->HasTryCatch()) {
// 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 0659c0beb5..4317b5d0ba 100644
--- a/test/004-ReferenceMap/stack_walk_refmap_jni.cc
+++ b/test/004-ReferenceMap/stack_walk_refmap_jni.cc
@@ -52,6 +52,7 @@ 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 c39274b625..bda978eb2c 100644
--- a/test/061-out-of-memory/src/Main.java
+++ b/test/061-out-of-memory/src/Main.java
@@ -108,7 +108,7 @@ public class Main {
System.out.println("testOomeSmall succeeded");
}
- private static Object testOomeToCharArray() {
+ private static void testOomeToCharArray() {
Object[] o = new Object[2000000];
String test = "test";
int i = 0;
@@ -123,6 +123,5 @@ public class Main {
o = null;
System.out.println("Got expected toCharArray OOM");
}
- return o;
}
}
diff --git a/test/530-checker-lse-try-catch/expected-stderr.txt b/test/530-checker-lse-try-catch/expected-stderr.txt
deleted file mode 100644
index e69de29bb2..0000000000
--- a/test/530-checker-lse-try-catch/expected-stderr.txt
+++ /dev/null
diff --git a/test/530-checker-lse-try-catch/expected-stdout.txt b/test/530-checker-lse-try-catch/expected-stdout.txt
deleted file mode 100644
index e69de29bb2..0000000000
--- a/test/530-checker-lse-try-catch/expected-stdout.txt
+++ /dev/null
diff --git a/test/530-checker-lse-try-catch/info.txt b/test/530-checker-lse-try-catch/info.txt
deleted file mode 100644
index 9ceef8275a..0000000000
--- a/test/530-checker-lse-try-catch/info.txt
+++ /dev/null
@@ -1 +0,0 @@
-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
deleted file mode 100644
index c7db02a668..0000000000
--- a/test/530-checker-lse-try-catch/src/Main.java
+++ /dev/null
@@ -1,207 +0,0 @@
-/*
- * 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 5e465f2716..743d50148c 100644
--- a/test/639-checker-code-sinking/src/Main.java
+++ b/test/639-checker-code-sinking/src/Main.java
@@ -679,11 +679,6 @@ 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;
}