diff options
| author | 2022-04-08 09:02:05 +0000 | |
|---|---|---|
| committer | 2022-04-08 09:07:54 +0000 | |
| commit | 2913ddad8b7f6e2ff7c92347ee8f67346cdfb16f (patch) | |
| tree | 744d6ca4b0e60debfb8abe47592bc56e4269f7b6 | |
| parent | fb95eedb3c8a383ea94868f5da14b5ea9b69a87d (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.cc | 12 | ||||
| -rw-r--r-- | test/004-ReferenceMap/stack_walk_refmap_jni.cc | 1 | ||||
| -rw-r--r-- | test/061-out-of-memory/src/Main.java | 3 | ||||
| -rw-r--r-- | test/530-checker-lse-try-catch/expected-stderr.txt | 0 | ||||
| -rw-r--r-- | test/530-checker-lse-try-catch/expected-stdout.txt | 0 | ||||
| -rw-r--r-- | test/530-checker-lse-try-catch/info.txt | 1 | ||||
| -rw-r--r-- | test/530-checker-lse-try-catch/src/Main.java | 207 | ||||
| -rw-r--r-- | test/639-checker-code-sinking/src/Main.java | 5 |
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; } |