Refine where stores are visible in LSE even more
In aosp/2219242 we allowed LSE to run in catch blocks but when
setting the restrictions we were overly restrictive and we
weren't allowing LSE to run in catches where we should have e.g.
try {
...
} catch (ArrayIndexOutOfBoundsException expected) {
... LSE can now run here for some instructions ...
try {
...
} catch (ArrayIndexOutOfBoundsException expectedToo) {
...
}
}
If a catch block starts a try we would have something like
Nop
LoadException
ClearException
The `... LSE can now run here ...` instructions above
TryBoundary kind:entry
and it would have xhandlers. The instructions before the
TryBoundary kind:entry are not considered to be part of a try and
they behave like normal code would.
As a note if code that looks like:
try {
try {
...
} catch (ArrayIndexOutOfBoundsException expectedToo) {
... throwing instructions here ...
}
} catch (ArrayIndexOutOfBoundsException expected) {
...
}
the `throwing instructions here` will not be part of the catch block.
Instead, we would have
Nop
LoadException
ClearException
TryBoundary kind:entry
// Next block, within the TryBoundary and considered part of a try
The `... throwing instructions here ...` instructions
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Bug: 227283233
Change-Id: I6b0cf66bc710a9009514f59076ae3ef0bbc287b7
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 4862f58..f520f9c 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -1088,25 +1088,9 @@
VisitSetLocation(instruction, idx, instruction->GetValue());
}
- static bool IsBlockInsideATry(HBasicBlock* block) {
- TryCatchInformation* try_catch_info = block->GetTryCatchInformation();
- if (try_catch_info == nullptr) {
- return false;
- }
-
- if (try_catch_info->IsTryBlock()) {
- return true;
- }
-
- DCHECK(try_catch_info->IsCatchBlock());
-
- // The catch block has an xhandler iff it is inside of an outer try.
- return block->GetExceptionalSuccessors().size() != 0;
- }
-
void VisitDeoptimize(HDeoptimize* instruction) override {
// If we are in a try, even singletons are observable.
- const bool inside_a_try = IsBlockInsideATry(instruction->GetBlock());
+ const bool inside_a_try = instruction->GetBlock()->IsTryBlock();
HBasicBlock* block = instruction->GetBlock();
ScopedArenaVector<ValueRecord>& heap_values = heap_values_for_[block->GetBlockId()];
for (size_t i = 0u, size = heap_values.size(); i != size; ++i) {
@@ -1165,7 +1149,7 @@
void HandleThrowingInstruction(HInstruction* instruction) {
DCHECK(instruction->CanThrow());
// If we are inside of a try, singletons can become visible since we may not exit the method.
- HandleExit(instruction->GetBlock(), IsBlockInsideATry(instruction->GetBlock()));
+ HandleExit(instruction->GetBlock(), instruction->GetBlock()->IsTryBlock());
}
void VisitMethodEntryHook(HMethodEntryHook* method_entry) override {
@@ -1224,8 +1208,7 @@
// If `instruction` can throw we have to presume all stores are visible.
const bool can_throw = instruction->CanThrow();
// If we are in a try, even singletons are observable.
- const bool can_throw_inside_a_try =
- can_throw && IsBlockInsideATry(instruction->GetBlock());
+ const bool can_throw_inside_a_try = can_throw && instruction->GetBlock()->IsTryBlock();
SideEffects side_effects = instruction->GetSideEffects();
ScopedArenaVector<ValueRecord>& heap_values =
heap_values_for_[instruction->GetBlock()->GetBlockId()];
@@ -1301,7 +1284,7 @@
void VisitNewInstance(HNewInstance* new_instance) override {
// If we are in a try, even singletons are observable.
- const bool inside_a_try = IsBlockInsideATry(new_instance->GetBlock());
+ const bool inside_a_try = new_instance->GetBlock()->IsTryBlock();
ReferenceInfo* ref_info = heap_location_collector_.FindReferenceInfoOf(new_instance);
if (ref_info == nullptr) {
// new_instance isn't used for field accesses. No need to process it.
@@ -1340,7 +1323,7 @@
void VisitNewArray(HNewArray* new_array) override {
// If we are in a try, even singletons are observable.
- const bool inside_a_try = IsBlockInsideATry(new_array->GetBlock());
+ const bool inside_a_try = new_array->GetBlock()->IsTryBlock();
ReferenceInfo* ref_info = heap_location_collector_.FindReferenceInfoOf(new_array);
if (ref_info == nullptr) {
// new_array isn't used for array accesses. No need to process it.
diff --git a/test/530-checker-lse-try-catch/src/Main.java b/test/530-checker-lse-try-catch/src/Main.java
index 68c877b..3f4fa25 100644
--- a/test/530-checker-lse-try-catch/src/Main.java
+++ b/test/530-checker-lse-try-catch/src/Main.java
@@ -35,6 +35,14 @@
assertEquals(30, $noinline$testDontKeepStoreInsideCatch(new int[]{}));
assertEquals(10, $noinline$testKeepStoreInsideCatchWithOuterTry(new int[]{10}));
assertEquals(30, $noinline$testKeepStoreInsideCatchWithOuterTry(new int[]{}));
+ assertEquals(40, $noinline$testDontKeepStoreInsideFinally(new int[]{10}));
+ try {
+ assertEquals(30, $noinline$testDontKeepStoreInsideFinally(new int[]{}));
+ throw new Error("Unreachable");
+ } catch (ArrayIndexOutOfBoundsException expected) {
+ }
+ assertEquals(10, $noinline$testDontKeepStoreInsideOuterCatch(new int[]{10}));
+ assertEquals(100030, $noinline$testDontKeepStoreInsideOuterCatch(new int[]{}));
assertEquals(150, $noinline$test40());
}
@@ -284,6 +292,66 @@
return main.sumForKeepStoreInsideTryCatch + value;
}
+ // Note that there are four `InstanceFieldSet` instead of two since we split the `finally` block
+ // into the normal path, and the exceptional path.
+
+ /// CHECK-START: int Main.$noinline$testDontKeepStoreInsideFinally(int[]) load_store_elimination (before)
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK-NOT: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+
+ /// CHECK-START: int Main.$noinline$testDontKeepStoreInsideFinally(int[]) load_store_elimination (after)
+ /// CHECK-NOT: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ private static int $noinline$testDontKeepStoreInsideFinally(int[] array) {
+ Main main = new Main();
+ int value = 0;
+ try {
+ value = array[0];
+ } finally {
+ // These sets can be eliminated even though we have invokes since this catch is not part of an
+ // outer try.
+ main.sumForKeepStoreInsideTryCatch += $noinline$returnValue(10);
+ main.sumForKeepStoreInsideTryCatch += $noinline$returnValue(20);
+ }
+ return main.sumForKeepStoreInsideTryCatch + value;
+ }
+
+ // Checks that we are able to do LSE inside of catches which are outside of try blocks.
+
+ /// CHECK-START: int Main.$noinline$testDontKeepStoreInsideOuterCatch(int[]) load_store_elimination (before)
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK-NOT: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+
+ // This store potentially can be eliminated too, but our phi creation logic doesn't realize it can
+ // create a Phi for `main.sumForKeepStoreInsideTryCatch` and skip a store+load.
+
+ /// CHECK-START: int Main.$noinline$testDontKeepStoreInsideOuterCatch(int[]) load_store_elimination (after)
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK-NOT: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+
+ private static int $noinline$testDontKeepStoreInsideOuterCatch(int[] array) {
+ Main main = new Main();
+ int value = 0;
+ try {
+ value = array[0];
+ } catch (ArrayIndexOutOfBoundsException expected) {
+ // These sets and gets are not considered to be part of a try so they are free to be
+ // eliminated.
+ main.sumForKeepStoreInsideTryCatch += $noinline$returnValue(10);
+ main.sumForKeepStoreInsideTryCatch += $noinline$returnValue(20);
+ try {
+ value = array[0];
+ } catch (ArrayIndexOutOfBoundsException expectedToo) {
+ value = 100000;
+ }
+ }
+
+ return main.sumForKeepStoreInsideTryCatch + value;
+ }
+
/// CHECK-START: int Main.$noinline$test40() load_store_elimination (before)
/// CHECK: ArraySet
/// CHECK: DivZeroCheck